Skip to content

Commit

Permalink
Preserve URL fragment during SAML handshake.
Browse files Browse the repository at this point in the history
  • Loading branch information
azasypkin committed Sep 2, 2019
1 parent 8e49146 commit 71f2980
Show file tree
Hide file tree
Showing 21 changed files with 626 additions and 258 deletions.
2 changes: 1 addition & 1 deletion docs/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: [/security/api/authc/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 Down
16 changes: 16 additions & 0 deletions renovate.json5
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,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
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')),
});

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
52 changes: 21 additions & 31 deletions x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,27 @@ import { KibanaRequest } from '../../../../../../../../src/core/server';
import { createCSPRuleString } from '../../../../../../../../src/legacy/server/csp';

export function initAuthenticateApi({ authc: { login, logout }, config }, server) {
const xsrfWhitelist = server.config().get('server.xsrf.whitelist');
server.ext('onRequest', (request, h) => {
// We handle this route in new platform plugin, but use different URL.
if (request.path === '/api/security/v1/saml') {
server.log(
['warning', 'security', 'deprecation'],
`The following URL is deprecated and will stop working in the next major version: ${request.path}`
);

// If deprecated route (`/api/security/v1/saml`) is included into XSRF whitelist we should
// implicitly include `/security/api/authc/saml/callback` as well to preserve the same behavior.
// We can't modify config, but we can mark route as ^safe^ via `kbn-xsrf` header.
if (xsrfWhitelist.includes(request.path)) {
request.headers['kbn-xsrf'] = true;
}

request.setUrl('/security/api/authc/saml/callback');
}

return h.continue;
});

server.route({
method: 'POST',
Expand Down Expand Up @@ -52,37 +73,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 @@ -104,6 +104,8 @@
"@types/tar-fs": "^1.16.1",
"@types/tinycolor2": "^1.4.1",
"@types/uuid": "^3.4.4",
"@types/xml2js": "^0.4.4",
"@types/xml-crypto": "^1.4.0",
"abab": "^1.0.4",
"ansicolors": "0.3.2",
"axios": "^0.19.0",
Expand Down
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 {
ClusterClient,
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';
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
mockScopedClusterClient,
} from './base.mock';

import { SAMLAuthenticationProvider } from './saml';
import { SAMLAuthenticationProvider, SAMLLoginStep } from './saml';

describe('SAMLAuthenticationProvider', () => {
let provider: SAMLAuthenticationProvider;
Expand Down Expand Up @@ -49,8 +49,12 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ requestId: 'some-request-id', nextURL: '/test-base-path/some-path' }
{
step: SAMLLoginStep.SAMLResponseReceived,
samlResponse: 'saml-response-xml',
nextURL: '/test-base-path/some-path',
},
{ requestId: 'some-request-id' }
);

sinon.assert.calledWithExactly(
Expand All @@ -72,8 +76,12 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ nextURL: '/test-base-path/some-path' }
{
step: SAMLLoginStep.SAMLResponseReceived,
samlResponse: 'saml-response-xml',
nextURL: '/test-base-path/some-path',
},
{}
);

sinon.assert.notCalled(mockOptions.client.callAsInternalUser);
Expand All @@ -91,7 +99,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
{ requestId: 'some-request-id' }
);

Expand All @@ -114,6 +122,7 @@ describe('SAMLAuthenticationProvider', () => {
});

const authenticationResult = await provider.login(request, {
step: SAMLLoginStep.SAMLResponseReceived,
samlResponse: 'saml-response-xml',
});

Expand Down Expand Up @@ -141,8 +150,12 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ requestId: 'some-request-id', nextURL: '/test-base-path/some-path' }
{
step: SAMLLoginStep.SAMLResponseReceived,
samlResponse: 'saml-response-xml',
nextURL: '/test-base-path/some-path',
},
{ requestId: 'some-request-id' }
);

sinon.assert.calledWithExactly(
Expand Down Expand Up @@ -171,7 +184,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
{ accessToken: 'some-valid-token', refreshToken: 'some-valid-refresh-token' }
);

Expand Down Expand Up @@ -212,7 +225,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
{ accessToken: 'existing-valid-token', refreshToken: 'existing-valid-refresh-token' }
);

Expand Down Expand Up @@ -247,7 +260,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
tokenPair
);

Expand Down Expand Up @@ -281,7 +294,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
tokenPair
);

Expand Down Expand Up @@ -329,7 +342,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
tokenPair
);

Expand Down Expand Up @@ -377,7 +390,7 @@ describe('SAMLAuthenticationProvider', () => {

const authenticationResult = await provider.login(
request,
{ samlResponse: 'saml-response-xml' },
{ step: SAMLLoginStep.SAMLResponseReceived, samlResponse: 'saml-response-xml' },
tokenPair
);

Expand Down
Loading

0 comments on commit 71f2980

Please sign in to comment.