From 4d13e5d03b7152826221a1076a3e22786747eadb Mon Sep 17 00:00:00 2001 From: Jakub Wachowski Date: Mon, 24 Oct 2016 15:56:49 +0200 Subject: [PATCH] Fix problems with multitenancy This commit fixes the following problems: - Kibana is not auto-detected by monasca-agent (Bug-Id: 13118) - Kibana renders error page when token expires - Keystone-token can be 'stolen' due to sharing of default session in Kibana - Some endpoints were not secured Related change for monasca-ui: https://review.openstack.org/#/c/387269/ Bug-Id: 13118 Change-Id: I2a363ebabcf593469943f7d071a92a680344ec73 --- package.json | 2 +- server/__tests__/retrieveToken.spec.js | 38 +++++++++++++++++------- server/mt/auth/token/index.js | 8 +++++ server/mt/auth/verify.js | 2 +- server/mt/index.js | 2 +- server/mt/routing/routes/kibana_index.js | 2 +- server/mt/routing/routes/mget.js | 2 +- server/mt/routing/routes/paths.js | 10 +++++-- 8 files changed, 48 insertions(+), 18 deletions(-) diff --git a/package.json b/package.json index ff93b9a..2db9f31 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fts-keystone", - "version": "0.0.3", + "version": "0.0.4", "description": "Keystone authentication & multitenancy support for Kibana 4.4.x", "author": "Fujitsu Enabling Software Technology GmbH", "license": "Apache-2.0", diff --git a/server/__tests__/retrieveToken.spec.js b/server/__tests__/retrieveToken.spec.js index 7721c5f..5c99ec4 100644 --- a/server/__tests__/retrieveToken.spec.js +++ b/server/__tests__/retrieveToken.spec.js @@ -27,13 +27,22 @@ describe('plugins/fts-keystone', ()=> { let server; beforeEach(()=> { + let configGet = sinon.stub(); + configGet.withArgs('fts-keystone.cookie.name').returns('keystone'); server = { - log: sinon.stub() + log: sinon.stub(), + config: function () { + return { + get: configGet + }; + } }; }); it('should return isBoom if session not available', ()=> { - let request = {}; + let request = { + state: {} + }; let errMsg = /Session support is missing/; chai.expect(()=> { @@ -41,14 +50,16 @@ describe('plugins/fts-keystone', ()=> { }).to.throw(errMsg); request = { - yar: undefined + yar: undefined, + state: {} }; chai.expect(()=> { retrieveToken(server, request); }).to.throw(errMsg); request = { - session: null + session: null, + state: {} }; chai.expect(()=> { retrieveToken(server, request); @@ -62,9 +73,11 @@ describe('plugins/fts-keystone', ()=> { 'get': sinon .stub() .withArgs(CONSTANTS.SESSION_TOKEN_KEY) - .returns(undefined) + .returns(undefined), + 'reset': sinon.stub() }, - headers: {} + headers: {}, + state: {} }; let result = retrieveToken(server, request); @@ -82,7 +95,8 @@ describe('plugins/fts-keystone', ()=> { }; let request = { yar : yar, - headers: {} + headers: {}, + state: {} }; let token; @@ -102,7 +116,7 @@ describe('plugins/fts-keystone', ()=> { it('should set token in session if not there and request has it', () => { let expectedToken = 'SOME_RANDOM_TOKEN'; let yar = { - 'reset': sinon.spy(), + 'reset': sinon.stub(), 'set' : sinon.spy(), 'get' : sinon.stub() }; @@ -110,7 +124,8 @@ describe('plugins/fts-keystone', ()=> { yar : yar, headers: { 'x-auth-token': expectedToken - } + }, + state: {} }; let token; @@ -158,14 +173,15 @@ describe('plugins/fts-keystone', ()=> { let token; let request = { yar : yar, - headers: headers + headers: headers, + state: {} }; token = retrieveToken(server, request); chai.expect(token).to.not.be.undefined; chai.expect(token).to.be.eql(RELOAD_SYMBOL); - chai.expect(yar.reset.calledOnce).to.be.ok; + chai.expect(yar.reset.calledTwice).to.be.ok; chai.expect(yar.get.calledOnce).to.be.ok; chai.expect(yar.set.callCount).to.be.eq(2); diff --git a/server/mt/auth/token/index.js b/server/mt/auth/token/index.js index 2bbcdbe..4fdcc72 100644 --- a/server/mt/auth/token/index.js +++ b/server/mt/auth/token/index.js @@ -43,6 +43,14 @@ module.exports = (server, request) => { throw new Error('Session support is missing'); } + // this is a workaround for problem with 'default' session: + // when there is no session cookie present, then yar uses default session, + // as a result many clients use the same session - security risk! + const cookieName = server.config().get('fts-keystone.cookie.name'); + if (!request.state[cookieName]) { + request.yar.reset(); + } + // DEV PURPOSE ONLY // request.yar.set(SESSION_TOKEN_KEY, 'a60e832483c34526a0c2bc3c6f8fa320'); diff --git a/server/mt/auth/verify.js b/server/mt/auth/verify.js index 055da34..7e871cf 100644 --- a/server/mt/auth/verify.js +++ b/server/mt/auth/verify.js @@ -42,7 +42,7 @@ export default () => { if (diff >= 0) { session.reset(); - return reply(Boom.unauthorized('User token has expired')).takeover(); + return reply(Boom.unauthorized('User token has expired')); } else { return reply.continue({ credentials: userObj, diff --git a/server/mt/index.js b/server/mt/index.js index 7c03b3a..8ce8f7b 100644 --- a/server/mt/index.js +++ b/server/mt/index.js @@ -46,7 +46,7 @@ function bindAuthScheme(server) { server.auth.strategy( 'session', 'keystone-token', - true, + false, require('./auth/strategy')(server) ) ]); diff --git a/server/mt/routing/routes/kibana_index.js b/server/mt/routing/routes/kibana_index.js index d1ede83..b72c535 100644 --- a/server/mt/routing/routes/kibana_index.js +++ b/server/mt/routing/routes/kibana_index.js @@ -27,7 +27,7 @@ export default function (server, method, path) { method : method, path : path, config : { - auth : false, + auth : 'session', payload: { output: 'data', parse : false diff --git a/server/mt/routing/routes/mget.js b/server/mt/routing/routes/mget.js index 5dfade5..a18d68c 100644 --- a/server/mt/routing/routes/mget.js +++ b/server/mt/routing/routes/mget.js @@ -26,7 +26,7 @@ export default function (server, method, path) { method : method, path : path, config : { - auth : false, + auth : 'session', payload: { output: 'data', parse : false diff --git a/server/mt/routing/routes/paths.js b/server/mt/routing/routes/paths.js index 4622d22..60e7fc7 100644 --- a/server/mt/routing/routes/paths.js +++ b/server/mt/routing/routes/paths.js @@ -13,6 +13,7 @@ */ import Wreck from 'wreck'; +import Boom from 'boom'; import { SESSION_USER_KEY } from '../../../const'; import { getOpts } from '../_utils'; @@ -20,14 +21,15 @@ import kibanaIndex from '../../kibana/kibanaIndex'; import mapUri from '../_map_uri'; export default function (server, method, path) { - const defaultKibanaIndex = defaultKibanaIndex; + const defaultKibanaIndex = server.config().get('kibana.index'); + const logIndexPostionInUrl = 3; return { method : method, path : path, config : { tags: ['elasticsearch', 'multitenancy'], - auth: false + auth: 'session' }, handler: handler }; @@ -42,7 +44,11 @@ export default function (server, method, path) { if (indexPos > -1) { url[indexPos] = kibanaIndex(server, session[SESSION_USER_KEY]); kibanaIndexRequest = true; + } else if (url.length > logIndexPostionInUrl + && !url[logIndexPostionInUrl].startsWith(session[SESSION_USER_KEY].project.id)) { + return reply(Boom.unauthorized('User does not have access to this resource')); } + url = url.join('/'); const opts = getOpts(server, request, url);