Skip to content

Commit

Permalink
Fix: Canvas socket auth (#24094)
Browse files Browse the repository at this point in the history
## Summary

Closes #23303 ~(@cqliu1 can you confirm this too?)~ confirmed

Fixes the way we capture the request info when configuring the socket and providing it to plugins via `callWithRequest`. Instead of exposing a route that returns the info, simply use the request object that comes back from `server.inject`.

Also adds a check in the `elasticsearchClient` handler exposed to plugins to ensure the session is still valid because using `callWithRequest`.

![screenshot 2018-10-16 10 37 56](https://user-images.githubusercontent.com/404731/47036828-32768c00-d132-11e8-81a0-122b5e83c7ef.png)
*Note:* the actual error message is a bit different, but this is how the failure is exposed to the user
  • Loading branch information
w33ble committed Oct 24, 2018
1 parent 0522fc7 commit fe7c9ec
Show file tree
Hide file tree
Showing 7 changed files with 147 additions and 77 deletions.
103 changes: 103 additions & 0 deletions x-pack/plugins/canvas/server/lib/__tests__/create_handlers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* 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 expect from 'expect.js';
import { createHandlers } from '../create_handlers';

let securityMode = 'pass';
const authError = new Error('auth error');

const mockRequest = {
headers: 'i can haz headers',
};

const mockServer = {
plugins: {
security: {
authenticate: () => ({
succeeded: () => (securityMode === 'pass' ? true : false),
error: securityMode === 'pass' ? null : authError,
}),
},
elasticsearch: {
getCluster: () => ({
callWithRequest: (...args) => Promise.resolve(args),
}),
},
},
config: () => ({
has: () => false,
get: val => val,
}),
info: {
uri: 'serveruri',
},
};

describe('server createHandlers', () => {
let handlers;

beforeEach(() => {
securityMode = 'pass';
handlers = createHandlers(mockRequest, mockServer);
});

it('provides helper methods and properties', () => {
expect(handlers).to.have.property('environment', 'server');
expect(handlers).to.have.property('serverUri');
expect(handlers).to.have.property('httpHeaders', mockRequest.headers);
expect(handlers).to.have.property('elasticsearchClient');
});

describe('elasticsearchClient', () => {
it('executes callWithRequest', async () => {
const [request, endpoint, payload] = await handlers.elasticsearchClient(
'endpoint',
'payload'
);
expect(request).to.equal(mockRequest);
expect(endpoint).to.equal('endpoint');
expect(payload).to.equal('payload');
});

it('rejects when authentication check fails', () => {
securityMode = 'fail';
return handlers
.elasticsearchClient('endpoint', 'payload')
.then(() => {
throw new Error('elasticsearchClient should fail when authentication fails');
})
.catch(err => {
// note: boom pre-pends error messages with "Error: "
expect(err.message).to.be.equal(`Error: ${authError.message}`);
});
});

it('works without security', async () => {
// create server without security plugin
const mockServerClone = {
...mockServer,
plugins: { ...mockServer.plugins },
};
delete mockServerClone.plugins.security;
expect(mockServer.plugins).to.have.property('security'); // confirm original server object
expect(mockServerClone.plugins).to.not.have.property('security');

// this shouldn't do anything
securityMode = 'fail';

// make sure the method still works
handlers = createHandlers(mockRequest, mockServerClone);
const [request, endpoint, payload] = await handlers.elasticsearchClient(
'endpoint',
'payload'
);
expect(request).to.equal(mockRequest);
expect(endpoint).to.equal('endpoint');
expect(payload).to.equal('payload');
});
});
});
12 changes: 10 additions & 2 deletions x-pack/plugins/canvas/server/lib/create_handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { partial } from 'lodash';
import boom from 'boom';

export const createHandlers = (request, server) => {
const { callWithRequest } = server.plugins.elasticsearch.getCluster('data');
Expand All @@ -17,6 +17,14 @@ export const createHandlers = (request, server) => {
? `${server.info.uri}${config.get('server.basePath')}`
: server.info.uri,
httpHeaders: request.headers,
elasticsearchClient: partial(callWithRequest, request),
elasticsearchClient: async (...args) => {
// check if the session is valid because continuing to use it
if (server.plugins.security) {
const authenticationResult = await server.plugins.security.authenticate(request);
if (!authenticationResult.succeeded()) throw boom.unauthorized(authenticationResult.error);
}

return callWithRequest(request, ...args);
},
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/

import uuid from 'uuid/v4';
export function getRequest(server, { headers }) {
const basePath = server.config().get('server.basePath') || '/';

export const insecureAuthRoute = `/api/canvas/ar-${uuid()}`;
return server
.inject({
method: 'GET',
url: basePath,
headers,
})
.then(res => res.request);
}
21 changes: 0 additions & 21 deletions x-pack/plugins/canvas/server/routes/get_auth/get_auth_header.js

This file was deleted.

22 changes: 0 additions & 22 deletions x-pack/plugins/canvas/server/routes/get_auth/index.js

This file was deleted.

2 changes: 0 additions & 2 deletions x-pack/plugins/canvas/server/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { socketApi } from './socket';
import { translate } from './translate';
import { esFields } from './es_fields';
import { esIndices } from './es_indices';
import { getAuth } from './get_auth';
import { plugins } from './plugins';

export function routes(server) {
Expand All @@ -18,6 +17,5 @@ export function routes(server) {
translate(server);
esFields(server);
esIndices(server);
getAuth(server);
plugins(server);
}
52 changes: 24 additions & 28 deletions x-pack/plugins/canvas/server/routes/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,12 @@ import { serializeProvider } from '../../common/lib/serialize';
import { functionsRegistry } from '../../common/lib/functions_registry';
import { typesRegistry } from '../../common/lib/types_registry';
import { loadServerPlugins } from '../lib/load_server_plugins';
import { getAuthHeader } from './get_auth/get_auth_header';
import { getRequest } from '../lib/get_request';

export function socketApi(server) {
const io = socket(server.listener, { path: '/socket.io' });

io.on('connection', socket => {
// This is the HAPI request object
const request = socket.handshake;

const authHeader = getAuthHeader(request, server);

// Create the function list
socket.emit('getFunctionList');
const getClientFunctions = new Promise(resolve => socket.once('functionList', resolve));
Expand All @@ -31,30 +26,31 @@ export function socketApi(server) {
});

const handler = ({ ast, context, id }) => {
Promise.all([getClientFunctions, authHeader]).then(([clientFunctions, authHeader]) => {
if (server.plugins.security) request.headers.authorization = authHeader;

const types = typesRegistry.toJS();
const interpret = socketInterpreterProvider({
types,
functions: functionsRegistry.toJS(),
handlers: createHandlers(request, server),
referableFunctions: clientFunctions,
socket: socket,
});
Promise.all([getClientFunctions, getRequest(server, socket.handshake)]).then(
([clientFunctions, request]) => {
// request is the modified hapi request object
const types = typesRegistry.toJS();
const interpret = socketInterpreterProvider({
types,
functions: functionsRegistry.toJS(),
handlers: createHandlers(request, server),
referableFunctions: clientFunctions,
socket: socket,
});

const { serialize, deserialize } = serializeProvider(types);
return interpret(ast, deserialize(context))
.then(value => {
socket.emit(`resp:${id}`, { value: serialize(value) });
})
.catch(e => {
socket.emit(`resp:${id}`, {
error: e.message,
stack: e.stack,
const { serialize, deserialize } = serializeProvider(types);
return interpret(ast, deserialize(context))
.then(value => {
socket.emit(`resp:${id}`, { value: serialize(value) });
})
.catch(e => {
socket.emit(`resp:${id}`, {
error: e.message,
stack: e.stack,
});
});
});
});
}
);
};

socket.on('run', handler);
Expand Down

0 comments on commit fe7c9ec

Please sign in to comment.