Skip to content

Commit

Permalink
Fixes unauthorized error in es datasources (#24624) (#24809)
Browse files Browse the repository at this point in the history
* Fixes check for security plugin

* Cleaned up security check logic. Fixed tests for create_handlers. Added TODOs

* Updated comment

* Added tests

* Updated variable names
  • Loading branch information
cqliu1 authored Oct 30, 2018
1 parent 348025a commit 1c7ec69
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 2 deletions.
51 changes: 50 additions & 1 deletion x-pack/plugins/canvas/server/lib/__tests__/create_handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import expect from 'expect.js';
import { createHandlers } from '../create_handlers';

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

const mockRequest = {
Expand All @@ -27,6 +29,15 @@ const mockServer = {
callWithRequest: (...args) => Promise.resolve(args),
}),
},
// TODO: remove this when we use the method exposed by security https://github.com/elastic/kibana/pull/24616
xpack_main: {
info: {
feature: () => ({
isAvailable: () => isSecurityAvailable,
isEnabled: () => isSecurityEnabled,
}),
},
},
},
config: () => ({
has: () => false,
Expand All @@ -42,6 +53,8 @@ describe('server createHandlers', () => {

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

Expand Down Expand Up @@ -76,7 +89,7 @@ describe('server createHandlers', () => {
});
});

it('works without security', async () => {
it('works without security plugin in kibana', async () => {
// create server without security plugin
const mockServerClone = {
...mockServer,
Expand All @@ -99,5 +112,41 @@ describe('server createHandlers', () => {
expect(endpoint).to.equal('endpoint');
expect(payload).to.equal('payload');
});

it('works without security available', async () => {
// create server with security unavailable (i.e. when user is on a basic license)
isSecurityAvailable = false;

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

// make sure the method still works
handlers = createHandlers(mockRequest, mockServer);
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('works with security disabled in elasticsearch', async () => {
// create server with security disabled
isSecurityEnabled = false;

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

// make sure the method still works
handlers = createHandlers(mockRequest, mockServer);
const [request, endpoint, payload] = await handlers.elasticsearchClient(
'endpoint',
'payload'
);
expect(request).to.equal(mockRequest);
expect(endpoint).to.equal('endpoint');
expect(payload).to.equal('payload');
});
});
});
4 changes: 3 additions & 1 deletion x-pack/plugins/canvas/server/lib/create_handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import boom from 'boom';
import { isSecurityEnabled } from './feature_check';

export const createHandlers = (request, server) => {
const { callWithRequest } = server.plugins.elasticsearch.getCluster('data');
Expand All @@ -19,7 +20,8 @@ export const createHandlers = (request, server) => {
httpHeaders: request.headers,
elasticsearchClient: async (...args) => {
// check if the session is valid because continuing to use it
if (server.plugins.security) {
// TODO: replace this when we use the method exposed by security https://github.com/elastic/kibana/pull/24616
if (isSecurityEnabled(server)) {
const authenticationResult = await server.plugins.security.authenticate(request);
if (!authenticationResult.succeeded()) throw boom.unauthorized(authenticationResult.error);
}
Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/canvas/server/lib/feature_check.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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.
*/

export const isSecurityEnabled = server => {
const kibanaSecurity = server.plugins.security;
const esSecurity = server.plugins.xpack_main.info.feature('security');

return kibanaSecurity && esSecurity.isAvailable() && esSecurity.isEnabled();
};

0 comments on commit 1c7ec69

Please sign in to comment.