From 4062fa62930597788c9483d7d77c112d8b9fc8c8 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Wed, 23 May 2018 17:13:03 -0400 Subject: [PATCH 1/6] POC for remembering last space --- x-pack/plugins/spaces/index.js | 5 ++++ .../server/lib/space_request_interceptors.js | 28 ++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/spaces/index.js b/x-pack/plugins/spaces/index.js index 603b8ddb22e11..ad3b7d634b0f6 100644 --- a/x-pack/plugins/spaces/index.js +++ b/x-pack/plugins/spaces/index.js @@ -76,6 +76,11 @@ export const spaces = (kibana) => new kibana.Plugin({ initSpacesApi(server); + server.state('selectedSpace', { + isHttpOnly: true, + path: config.get('server.basePath') + }); + initSpacesRequestInterceptors(server); await createDefaultSpace(server); diff --git a/x-pack/plugins/spaces/server/lib/space_request_interceptors.js b/x-pack/plugins/spaces/server/lib/space_request_interceptors.js index 6f4330ba283cc..a6cae98991226 100644 --- a/x-pack/plugins/spaces/server/lib/space_request_interceptors.js +++ b/x-pack/plugins/spaces/server/lib/space_request_interceptors.js @@ -48,11 +48,18 @@ export function initSpacesRequestInterceptors(server) { // which is not available at the time of "onRequest". if (isRequestingKibanaRoot && !urlContext) { try { + + const { selectedSpace } = request.state; + const client = request.getSavedObjectsClient(); const { total, saved_objects: spaceObjects } = await client.find({ type: 'space' }); + const config = server.config(); + const basePath = config.get('server.basePath'); + const defaultRoute = config.get('server.defaultRoute'); + if (total === 1) { // If only one space is available, then send user there directly. // No need for an interstitial screen where there is only one possible outcome. @@ -61,10 +68,6 @@ export function initSpacesRequestInterceptors(server) { urlContext } = space.attributes; - const config = server.config(); - const basePath = config.get('server.basePath'); - const defaultRoute = config.get('server.defaultRoute'); - let destination; if (urlContext) { destination = `${basePath}/s/${urlContext}${defaultRoute}`; @@ -76,6 +79,19 @@ export function initSpacesRequestInterceptors(server) { } if (total > 0) { + + const preferredSpace = spaceObjects.find(so => so.attributes.urlContext === selectedSpace); + if (preferredSpace) { + let destination; + if (preferredSpace.attributes.urlContext) { + destination = `${basePath}/s/${preferredSpace.attributes.urlContext}${defaultRoute}`; + } else { + destination = `${basePath}${defaultRoute}`; + } + + return reply.redirect(destination); + } + // render spaces selector instead of home page const app = server.getHiddenUiAppById('space_selector'); return reply.renderApp(app, { @@ -88,6 +104,10 @@ export function initSpacesRequestInterceptors(server) { } } + if (typeof urlContext === 'string' && request.state.selectedSpace !== urlContext) { + console.log('setting selectedSpace', urlContext); + reply.state('selectedSpace', urlContext); + } return reply.continue(); }); From 4e160af25519ec6624333e00212a9e441d797807 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 24 May 2018 13:55:01 -0400 Subject: [PATCH 2/6] Remember selected space post login, and anytime it's changed from the main menu --- .../client/saved_objects_client.js | 2 +- .../spaces_url_parser.test.js.snap | 3 + x-pack/plugins/spaces/common/constants.js | 7 + x-pack/plugins/spaces/common/index.js | 17 ++ .../spaces/common/spaces_url_parser.js | 13 +- .../spaces/common/spaces_url_parser.test.js | 72 ++++-- x-pack/plugins/spaces/index.js | 9 +- .../spaces/public/lib/spaces_manager.js | 10 + .../public/views/components/space_cards.js | 4 +- .../components/spaces_context_menu.js | 63 +++++ .../public/views/nav_control/nav_control.js | 4 +- .../public/views/nav_control/nav_control.less | 21 +- .../views/nav_control/nav_control_modal.js | 222 ++++++++-------- .../spaces/server/lib/get_active_space.js | 32 ++- .../spaces/server/lib/selected_space_state.js | 38 +++ .../server/lib/selected_space_state.test.js | 244 ++++++++++++++++++ .../server/lib/space_request_interceptors.js | 34 +-- .../lib/space_request_interceptors.test.js | 13 + .../spaces/server/routes/api/v1/spaces.js | 30 ++- 19 files changed, 654 insertions(+), 184 deletions(-) create mode 100644 x-pack/plugins/spaces/common/__snapshots__/spaces_url_parser.test.js.snap create mode 100644 x-pack/plugins/spaces/common/index.js create mode 100644 x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.js create mode 100644 x-pack/plugins/spaces/server/lib/selected_space_state.js create mode 100644 x-pack/plugins/spaces/server/lib/selected_space_state.test.js diff --git a/src/server/saved_objects/client/saved_objects_client.js b/src/server/saved_objects/client/saved_objects_client.js index 79dc55540e039..c1f4b65692718 100644 --- a/src/server/saved_objects/client/saved_objects_client.js +++ b/src/server/saved_objects/client/saved_objects_client.js @@ -16,7 +16,7 @@ export class SavedObjectsClient { index, mappings, callCluster, - onBeforeWrite = () => {}, + onBeforeWrite = () => { }, } = options; this._index = index; diff --git a/x-pack/plugins/spaces/common/__snapshots__/spaces_url_parser.test.js.snap b/x-pack/plugins/spaces/common/__snapshots__/spaces_url_parser.test.js.snap new file mode 100644 index 0000000000000..a42d029097b67 --- /dev/null +++ b/x-pack/plugins/spaces/common/__snapshots__/spaces_url_parser.test.js.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`addSpaceUrlContext it throws an error when the requested path does not start with a slash 1`] = `"path must start with a /"`; diff --git a/x-pack/plugins/spaces/common/constants.js b/x-pack/plugins/spaces/common/constants.js index 40e064ef98359..ca67766a9adb6 100644 --- a/x-pack/plugins/spaces/common/constants.js +++ b/x-pack/plugins/spaces/common/constants.js @@ -5,3 +5,10 @@ */ export const DEFAULT_SPACE_ID = `default`; + +export const SELECTED_SPACE_COOKIE = 'selectedSpace'; + +/** + * Cookie expiration for the user's selected Space. (90 days) + */ +export const SELECTED_SPACE_COOKIE_TTL_MILLIS = 1000 * 60 * 60 * 24 * 90; diff --git a/x-pack/plugins/spaces/common/index.js b/x-pack/plugins/spaces/common/index.js new file mode 100644 index 0000000000000..0adba302d4681 --- /dev/null +++ b/x-pack/plugins/spaces/common/index.js @@ -0,0 +1,17 @@ +/* + * 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 { + addSpaceUrlContext, + stripSpaceUrlContext, + getSpaceUrlContext, +} from './spaces_url_parser'; + +export { + DEFAULT_SPACE_ID, + SELECTED_SPACE_COOKIE, + SELECTED_SPACE_COOKIE_TTL_MILLIS, +} from './constants'; diff --git a/x-pack/plugins/spaces/common/spaces_url_parser.js b/x-pack/plugins/spaces/common/spaces_url_parser.js index 075f09eea03a3..e4c41526997e2 100644 --- a/x-pack/plugins/spaces/common/spaces_url_parser.js +++ b/x-pack/plugins/spaces/common/spaces_url_parser.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -export function getSpaceUrlContext(basePath = '/', defaultContext = null) { +export function getSpaceUrlContext(basePath = '/', defaultContext = '') { // Look for `/s/space-url-context` in the base path const matchResult = basePath.match(/\/s\/([a-z0-9\-]+)/); @@ -42,3 +42,14 @@ export function stripSpaceUrlContext(basePath = '/') { return basePathWithoutSpace; } + +export function addSpaceUrlContext(basePath = '/', urlContext = '', requestedPath = '') { + if (requestedPath && !requestedPath.startsWith('/')) { + throw new Error(`path must start with a /`); + } + + if (urlContext) { + return `${basePath}/s/${urlContext}${requestedPath}`; + } + return `${basePath}${requestedPath}`; +} diff --git a/x-pack/plugins/spaces/common/spaces_url_parser.test.js b/x-pack/plugins/spaces/common/spaces_url_parser.test.js index ee0813b2f70ac..0b2c530a620c3 100644 --- a/x-pack/plugins/spaces/common/spaces_url_parser.test.js +++ b/x-pack/plugins/spaces/common/spaces_url_parser.test.js @@ -3,37 +3,61 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { stripSpaceUrlContext, getSpaceUrlContext } from './spaces_url_parser'; +import { stripSpaceUrlContext, getSpaceUrlContext, addSpaceUrlContext } from './spaces_url_parser'; -test('it removes the space url context from the base path when the space is not at the root', () => { - const basePath = `/foo/s/my-space`; - expect(stripSpaceUrlContext(basePath)).toEqual('/foo'); -}); +describe('stripSpaceUrlContext', () => { + test('it removes the space url context from the base path when the space is not at the root', () => { + const basePath = `/foo/s/my-space`; + expect(stripSpaceUrlContext(basePath)).toEqual('/foo'); + }); -test('it removes the space url context from the base path when the space is the root', () => { - const basePath = `/s/my-space`; - expect(stripSpaceUrlContext(basePath)).toEqual(''); -}); + test('it removes the space url context from the base path when the space is the root', () => { + const basePath = `/s/my-space`; + expect(stripSpaceUrlContext(basePath)).toEqual(''); + }); -test(`it doesn't change base paths without a space url context`, () => { - const basePath = `/this/is/a-base-path/ok`; - expect(stripSpaceUrlContext(basePath)).toEqual(basePath); -}); + test(`it doesn't change base paths without a space url context`, () => { + const basePath = `/this/is/a-base-path/ok`; + expect(stripSpaceUrlContext(basePath)).toEqual(basePath); + }); -test('it accepts no parameters', () => { - expect(stripSpaceUrlContext()).toEqual(''); -}); + test('it accepts no parameters', () => { + expect(stripSpaceUrlContext()).toEqual(''); + }); -test('it remove the trailing slash', () => { - expect(stripSpaceUrlContext('/')).toEqual(''); + test('it remove the trailing slash', () => { + expect(stripSpaceUrlContext('/')).toEqual(''); + }); }); -test('it identifies the space url context', () => { - const basePath = `/this/is/a/crazy/path/s/my-awesome-space-lives-here`; - expect(getSpaceUrlContext(basePath)).toEqual('my-awesome-space-lives-here'); +describe('getSpaceUrlContext', () => { + test('it identifies the space url context', () => { + const basePath = `/this/is/a/crazy/path/s/my-awesome-space-lives-here`; + expect(getSpaceUrlContext(basePath)).toEqual('my-awesome-space-lives-here'); + }); + + test('it handles base url without a space url context', () => { + const basePath = `/this/is/a/crazy/path/s`; + expect(getSpaceUrlContext(basePath)).toEqual(null); + }); }); -test('it handles base url without a space url context', () => { - const basePath = `/this/is/a/crazy/path/s`; - expect(getSpaceUrlContext(basePath)).toEqual(null); +describe('addSpaceUrlContext', () => { + test('handles no parameters', () => { + expect(addSpaceUrlContext()).toEqual(`/`); + }); + + test('it adds to the basePath correctly', () => { + expect(addSpaceUrlContext('/my/base/path', 'url-context')).toEqual('/my/base/path/s/url-context'); + }); + + test('it appends the requested path to the end of the url context', () => { + expect(addSpaceUrlContext('/base', 'context', '/final/destination')).toEqual('/base/s/context/final/destination'); + }); + + test('it throws an error when the requested path does not start with a slash', () => { + expect(() => { + addSpaceUrlContext('', '', 'foo'); + }).toThrowErrorMatchingSnapshot(); + }); }); diff --git a/x-pack/plugins/spaces/index.js b/x-pack/plugins/spaces/index.js index ad3b7d634b0f6..c7dc9801c4328 100644 --- a/x-pack/plugins/spaces/index.js +++ b/x-pack/plugins/spaces/index.js @@ -14,6 +14,7 @@ import { mirrorPluginStatus } from '../../server/lib/mirror_plugin_status'; import { getActiveSpace } from './server/lib/get_active_space'; import { wrapError } from './server/lib/errors'; import mappings from './mappings.json'; +import { initSelectedSpaceState } from './server/lib/selected_space_state'; export const spaces = (kibana) => new kibana.Plugin({ id: 'spaces', @@ -24,6 +25,7 @@ export const spaces = (kibana) => new kibana.Plugin({ config(Joi) { return Joi.object({ enabled: Joi.boolean().default(true), + rememberSelectedSpace: Joi.boolean().default(true), }).default(); }, @@ -52,7 +54,7 @@ export const spaces = (kibana) => new kibana.Plugin({ valid: true, space: await getActiveSpace(request.getSavedObjectsClient(), request.getBasePath()) }; - } catch(e) { + } catch (e) { vars.activeSpace = { valid: false, error: wrapError(e).output.payload @@ -76,10 +78,7 @@ export const spaces = (kibana) => new kibana.Plugin({ initSpacesApi(server); - server.state('selectedSpace', { - isHttpOnly: true, - path: config.get('server.basePath') - }); + initSelectedSpaceState(server, config); initSpacesRequestInterceptors(server); diff --git a/x-pack/plugins/spaces/public/lib/spaces_manager.js b/x-pack/plugins/spaces/public/lib/spaces_manager.js index 77b1ea9b514ca..d7db211c28a3f 100644 --- a/x-pack/plugins/spaces/public/lib/spaces_manager.js +++ b/x-pack/plugins/spaces/public/lib/spaces_manager.js @@ -35,4 +35,14 @@ export class SpacesManager { return await this._httpAgent .delete(`${this._baseUrl}/space/${space.id}`); } + + async changeSelectedSpace(space) { + return await this._httpAgent + .put(`${this._baseUrl}/space/${space.id}/select`) + .then(response => { + if (response.data && response.data.location) { + window.location = response.data.location; + } + }); + } } diff --git a/x-pack/plugins/spaces/public/views/components/space_cards.js b/x-pack/plugins/spaces/public/views/components/space_cards.js index 2e92ce4a290ea..508c8ea53342d 100644 --- a/x-pack/plugins/spaces/public/views/components/space_cards.js +++ b/x-pack/plugins/spaces/public/views/components/space_cards.js @@ -8,7 +8,7 @@ import React, { Component, Fragment } from 'react'; import PropTypes from 'prop-types'; import chrome from 'ui/chrome'; import { SpaceCard } from './space_card'; -import { stripSpaceUrlContext } from '../../../common/spaces_url_parser'; +import { stripSpaceUrlContext, addSpaceUrlContext } from '../../../common/spaces_url_parser'; import { EuiFlexGroup, EuiFlexItem, @@ -48,7 +48,7 @@ export class SpaceCards extends Component { return () => { const baseUrlWithoutSpace = stripSpaceUrlContext(chrome.getBasePath()); - window.location = `${baseUrlWithoutSpace}/s/${space.urlContext}`; + window.location = addSpaceUrlContext(baseUrlWithoutSpace, space.urlContext); }; }; } diff --git a/x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.js b/x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.js new file mode 100644 index 0000000000000..603d0a00e8b2b --- /dev/null +++ b/x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.js @@ -0,0 +1,63 @@ +/* + * 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 React, { Component } from 'react'; +import PropTypes from 'prop-types'; +import { + EuiContextMenuPanel, + EuiContextMenuItem, + EuiFlexGroup, + EuiFlexItem, + EuiButton, + EuiText, +} from '@elastic/eui'; + +export class SpacesContextMenu extends Component { + static propTypes = { + spaces: PropTypes.array.isRequired, + onSelectSpace: PropTypes.func.isRequired, + showManageButton: PropTypes.bool + } + + render() { + const items = this.props.spaces.map(this.buildSpaceMenuOption); + + return ( + +

Select a Space

 + + {this.buildManageButton()} + + ); + } + + buildSpaceMenuOption = (space) => { + return ( + + {space.name} + + ); + } + + buildManageButton = () => { + if (!this.props.showManageButton) { + return null; + } + return ( + + { }} size={'s'}>Manage Spaces + + ); + } + + onSpaceClick = (space) => this.props.onSelectSpace.bind(this, space) +} diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control.js index 74be5a7f627dd..6c686a4a30064 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control.js @@ -23,12 +23,12 @@ chromeNavControlsRegistry.register(constant({ const module = uiModules.get('spaces', ['kibana']); -module.controller('spacesNavController', ($scope, $http, chrome, activeSpace) => { +module.controller('spacesNavController', ($scope, $http, chrome, globalNavState, activeSpace) => { const domNode = document.getElementById(`spacesNavReactRoot`); const spacesManager = new SpacesManager($http, chrome); - render(, domNode); + render(, domNode); // unmount react on controller destroy $scope.$on('$destroy', () => { diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control.less b/x-pack/plugins/spaces/public/views/nav_control/nav_control.less index 19f56c5ea1077..eb8569681c4a6 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control.less +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control.less @@ -1,8 +1,23 @@ +@import (reference) "~ui/styles/variables"; + .global-nav-link__icon .spaceNavGraphic { margin-top: 0.5em; } -.selectSpaceModal { - min-width: 450px; - max-width: 1200px; +.spaceSelectorMenu:last-child { + margin-bottom: 5px; +} + +.spaceSelectorMenu__manageSpacesButton { + margin: 0 5px; +} + +.spaceSelectorMenu__panel { + width: calc(@global-nav-open-width - 10px); +} + +.euiFlexItem.spaceSelectorMenu__title { + background-color: #fff; + padding: 10px; + margin-bottom: 0; } diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js index 1827d5bf8519d..d5ec849e75d0f 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js @@ -7,130 +7,134 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { - EuiModal, - EuiModalHeader, - EuiModalHeaderTitle, - EuiModalBody, - EuiOverlayMask, EuiAvatar, + EuiPopover, } from '@elastic/eui'; -import { SpaceCards } from '../components/space_cards'; import { Notifier } from 'ui/notify'; +import { SpacesContextMenu } from './components/spaces_context_menu'; export class NavControlModal extends Component { - state = { - isOpen: false, - loading: false, - spaces: [] - }; - - notifier = new Notifier(`Spaces`); - - async loadSpaces() { - const { - spacesManager - } = this.props; - - this.setState({ - loading: true - }); - - const spaces = await spacesManager.getSpaces(); - this.setState({ - spaces, - loading: false - }); - } - - componentDidMount() { - const { - activeSpace - } = this.props; - - if (activeSpace && !activeSpace.valid) { - const { error = {} } = activeSpace; - if (error.message) { - this.notifier.error(error.message); - } + buttonRef; + + state = { + isOpen: false, + loading: false, + spaces: [] + }; + + notifier = new Notifier(`Spaces`); + + async loadSpaces() { + console.log('loading spaces'); + const { + spacesManager + } = this.props; + + this.setState({ + loading: true + }); + + const spaces = await spacesManager.getSpaces(); + this.setState({ + spaces, + loading: false + }); + } + + componentDidMount() { + const { + activeSpace + } = this.props; + + if (activeSpace && !activeSpace.valid) { + const { error = {} } = activeSpace; + if (error.message) { + this.notifier.error(error.message); } } - - render() { - let modal; - if (this.state.isOpen) { - modal = ( - - - - Select a space - - - - - - - ); - } - - return ( -
{this.getActiveSpaceButton()}{modal}
- ); + } + + render() { + return ( + + + + ); + } + + getActiveSpaceButton = () => { + const { + activeSpace + } = this.props; + + if (!activeSpace) { + return null; } - getActiveSpaceButton = () => { - const { - activeSpace - } = this.props; - - if (!activeSpace) { - return null; - } - - if (activeSpace.valid && activeSpace.space) { - return this.getButton( - , - activeSpace.space.name - ); - } else if (activeSpace.error) { - return this.getButton( - , - 'error' - ); - } - - return null; - }; - - getButton = (linkIcon, linkTitle) => { - return ( - + if (activeSpace.valid && activeSpace.space) { + return this.getButton( + , + activeSpace.space.name ); - }; - - togglePortal = () => { - const isOpening = !this.state.isOpen; - if (isOpening) { - this.loadSpaces(); - } + } else if (activeSpace.error) { + return this.getButton( + , + 'error' + ); + } - this.setState({ - isOpen: !this.state.isOpen - }); - }; + return null; + }; + + getButton = (linkIcon, linkTitle) => { + return ( +
{ this.buttonRef = me; }} onClick={this.expandGlobalNav}> + +
{linkIcon}
+
{linkTitle}
+ +
+ ); + }; + + togglePortal = () => { + const isOpening = !this.state.isOpen; + if (isOpening) { + this.loadSpaces(); + } - closePortal = () => { - this.setState({ - isOpen: false - }); + this.setState({ + isOpen: !this.state.isOpen + }); + }; + + closePortal = () => { + this.setState({ + isOpen: false + }); + } + + expandGlobalNav = () => { + const { globalNavState } = this.props; + if (!globalNavState.isOpen()) { + globalNavState.setOpen(true); } + } + + onSelectSpace = (space) => { + this.props.spacesManager.changeSelectedSpace(space); + } } NavControlModal.propTypes = { activeSpace: PropTypes.object, - spacesManager: PropTypes.object.isRequired + spacesManager: PropTypes.object.isRequired, + globalNavState: PropTypes.object.isRequired, }; diff --git a/x-pack/plugins/spaces/server/lib/get_active_space.js b/x-pack/plugins/spaces/server/lib/get_active_space.js index 7fb7821e75274..69156a696f389 100644 --- a/x-pack/plugins/spaces/server/lib/get_active_space.js +++ b/x-pack/plugins/spaces/server/lib/get_active_space.js @@ -7,26 +7,19 @@ import Boom from 'boom'; import { wrapError } from './errors'; import { getSpaceUrlContext } from '../../common/spaces_url_parser'; +import { DEFAULT_SPACE_ID } from '../../common/constants'; export async function getActiveSpace(savedObjectsClient, basePath) { const spaceContext = getSpaceUrlContext(basePath); - if (!spaceContext) { + if (typeof spaceContext !== 'string') { return null; } let spaces; try { - const { - saved_objects: savedObjects - } = await savedObjectsClient.find({ - type: 'space', - search: `"${spaceContext}"`, - search_fields: ['urlContext'], - }); - - spaces = savedObjects || []; - } catch(e) { + spaces = await getSpacesFromContext(savedObjectsClient, spaceContext); + } catch (e) { throw wrapError(e); } @@ -49,3 +42,20 @@ export async function getActiveSpace(savedObjectsClient, basePath) { ...spaces[0].attributes }; } + +async function getSpacesFromContext(client, context) { + // Workaround for SOC's find operation, which can't "find" on an empty string. + if (!context) { + return [await client.get('space', DEFAULT_SPACE_ID)]; + } + + const { + saved_objects: savedObjects + } = await client.find({ + type: 'space', + search: `"${context}"`, + searchFields: ['urlContext'], + }); + + return savedObjects; +} diff --git a/x-pack/plugins/spaces/server/lib/selected_space_state.js b/x-pack/plugins/spaces/server/lib/selected_space_state.js new file mode 100644 index 0000000000000..7a7c8abbff992 --- /dev/null +++ b/x-pack/plugins/spaces/server/lib/selected_space_state.js @@ -0,0 +1,38 @@ +/* + * 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 { SELECTED_SPACE_COOKIE, SELECTED_SPACE_COOKIE_TTL_MILLIS } from '../../common'; + +let rememberSelectedSpace; + +export function initSelectedSpaceState(server, config) { + rememberSelectedSpace = config.get('xpack.spaces.rememberSelectedSpace'); + if (!rememberSelectedSpace) { + return; + } + + server.state(SELECTED_SPACE_COOKIE, { + ttl: SELECTED_SPACE_COOKIE_TTL_MILLIS, + isHttpOnly: true, + isSecure: config.get('server.ssl.enabled'), + path: config.get('server.basePath'), + }); +} + +export function setSelectedSpace(request, reply, urlContext) { + if (!rememberSelectedSpace) { + return; + } + + const { + [SELECTED_SPACE_COOKIE]: currentSelectedSpace + } = request.state; + + // a blank url context is different from undefined. Blank == the Default Space, while undefined means there is no space selected. + if (typeof urlContext === 'string' && urlContext !== currentSelectedSpace) { + reply.state(SELECTED_SPACE_COOKIE, urlContext); + } +} diff --git a/x-pack/plugins/spaces/server/lib/selected_space_state.test.js b/x-pack/plugins/spaces/server/lib/selected_space_state.test.js new file mode 100644 index 0000000000000..60ebc829917ff --- /dev/null +++ b/x-pack/plugins/spaces/server/lib/selected_space_state.test.js @@ -0,0 +1,244 @@ +/* + * 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 sinon from 'sinon'; +import { Server } from 'hapi'; +import { initSelectedSpaceState, setSelectedSpace } from './selected_space_state'; +import { SELECTED_SPACE_COOKIE } from '../../common'; + +function getCookiePartsFromResponse(response) { + const { headers } = response; + expect(headers).toHaveProperty('set-cookie'); + + const cookies = headers['set-cookie']; + expect(cookies).toHaveLength(1); + + return cookies[0].split(';').map(a => a.trim()); +} + +describe('initSelectedSpaceState', () => { + const sandbox = sinon.sandbox.create(); + const teardowns = []; + let request; + + beforeEach(() => { + teardowns.push(() => sandbox.restore()); + request = async (config, setupFn = () => { }) => { + + const server = new Server(); + + server.connection({ port: 0 }); + + initSelectedSpaceState(server, { + get: (key) => { + return config[key]; + } + }); + + server.route({ + method: 'GET', + path: '/', + handler: (req, reply) => { + return reply({ path: req.path, url: req.url }); + } + }); + + server.ext('onPreResponse', (req, reply) => { + reply.state(SELECTED_SPACE_COOKIE, 'foo'); + return reply.continue(); + }); + + await setupFn(server); + + teardowns.push(() => server.stop()); + + const response = await server.inject({ + method: 'GET', + url: '/', + }); + + if (response && response.isBoom) { + throw response; + } + + return response; + }; + }); + + afterEach(async () => { + await Promise.all(teardowns.splice(0).map(fn => fn())); + }); + + describe('Secure Flag', () => { + const values = [true, false]; + values.forEach(v => { + test(`it ${v ? 'sets' : 'does not set'} the Secure flag when server ssl is ${v ? 'enabled' : 'disabled'}`, async () => { + const config = { + 'server.ssl.enabled': v, + 'server.basePath': null, + 'xpack.spaces.rememberSelectedSpace': true, + }; + + const response = await request(config); + + const [nameValuePair, ...rest] = getCookiePartsFromResponse(response); + + expect(nameValuePair).toEqual('selectedSpace=foo'); + if (v) { + expect(rest).toContain('Secure'); + } else { + expect(rest).not.toContain('Secure'); + } + }); + }); + }); + + test('it sets the httpOnly flag', async () => { + const config = { + 'server.ssl.enabled': true, + 'server.basePath': null, + 'xpack.spaces.rememberSelectedSpace': true, + }; + + const response = await request(config); + const attributes = getCookiePartsFromResponse(response); + expect(attributes).toContain('HttpOnly'); + }); + + test('it sets the cookie path based on the servers basePath', async () => { + const config = { + 'server.ssl.enabled': true, + 'server.basePath': '/foo/bar', + 'xpack.spaces.rememberSelectedSpace': true, + }; + + const response = await request(config); + + const attributes = getCookiePartsFromResponse(response); + expect(attributes).toContain('Path=/foo/bar'); + }); +}); + +describe('setSelectedSpace', () => { + const sandbox = sinon.sandbox.create(); + const teardowns = []; + + const serverConfig = { + 'server.ssl.enabled': true, + 'server.basePath': null, + 'xpack.spaces.rememberSelectedSpace': true, + }; + + let request; + + beforeEach(() => { + teardowns.push(() => sandbox.restore()); + request = async (config, replyHandler = () => { }, reqHeaders = {}) => { + + const server = new Server(); + + server.connection({ port: 0 }); + + initSelectedSpaceState(server, { + get: (key) => { + return config[key]; + } + }); + + server.route({ + method: 'GET', + path: '/', + handler: (req, reply) => { + replyHandler(req, reply); + return reply({ path: req.path, url: req.url }); + } + }); + + teardowns.push(() => server.stop()); + + const response = await server.inject({ + method: 'GET', + url: '/', + headers: reqHeaders + }); + + if (response && response.isBoom) { + throw response; + } + return response; + }; + }); + + afterEach(async () => { + await Promise.all(teardowns.splice(0).map(fn => fn())); + }); + + test('it sets the selected space cookie', async () => { + const response = await request(serverConfig, (req, rep) => { + setSelectedSpace(req, rep, 'url-context'); + }); + + const [nameValuePair] = getCookiePartsFromResponse(response); + expect(nameValuePair).toEqual('selectedSpace=url-context'); + }); + + test('it does not set the selected space if it is already set to that value', async () => { + const requestHeaders = { + 'Cookie': 'selectedSpace=url-context' + }; + + const response = await request(serverConfig, (req, rep) => { + setSelectedSpace(req, rep, 'url-context'); + }, requestHeaders); + + const { headers } = response; + expect(headers).not.toHaveProperty('set-cookie'); + }); + + test('it overwrites the selected space if it has changed', async () => { + const requestHeaders = { + 'Cookie': 'selectedSpace=original-url-context' + }; + + const response = await request(serverConfig, (req, rep) => { + setSelectedSpace(req, rep, 'new-url-context'); + }, requestHeaders); + + const [nameValuePair] = getCookiePartsFromResponse(response); + expect(nameValuePair).toEqual('selectedSpace=new-url-context'); + }); + + test('it sets the selected space even if the url context is empty', async () => { + const response = await request(serverConfig, (req, rep) => { + setSelectedSpace(req, rep, ''); + }); + + const [nameValuePair] = getCookiePartsFromResponse(response); + expect(nameValuePair).toEqual('selectedSpace='); + }); + + test('it does not set the selected space if the url context is undefined', async () => { + const response = await request(serverConfig, (req, rep) => { + setSelectedSpace(req, rep); + }); + + const { headers } = response; + expect(headers).not.toHaveProperty('set-cookie'); + }); + + test('it does not set the selected space if the feature is turned off', async () => { + const config = { + ...serverConfig, + 'xpack.spaces.rememberSelectedSpace': false + }; + + const response = await request(config, (req, rep) => { + setSelectedSpace(req, rep, 'url-context'); + }); + + const { headers } = response; + expect(headers).not.toHaveProperty('set-cookie'); + }); +}); diff --git a/x-pack/plugins/spaces/server/lib/space_request_interceptors.js b/x-pack/plugins/spaces/server/lib/space_request_interceptors.js index a6cae98991226..db5e201980f5e 100644 --- a/x-pack/plugins/spaces/server/lib/space_request_interceptors.js +++ b/x-pack/plugins/spaces/server/lib/space_request_interceptors.js @@ -5,6 +5,8 @@ */ import { wrapError } from './errors'; +import { addSpaceUrlContext, SELECTED_SPACE_COOKIE } from '../../common'; +import { setSelectedSpace } from './selected_space_state'; export function initSpacesRequestInterceptors(server) { const contextCache = new WeakMap(); @@ -42,6 +44,7 @@ export function initSpacesRequestInterceptors(server) { const isRequestingKibanaRoot = path === '/'; const urlContext = contextCache.get(request); + const { [SELECTED_SPACE_COOKIE]: selectedSpace } = request.state; // if requesting the application root, then show the Space Selector UI to allow the user to choose which space // they wish to visit. This is done "onPostAuth" to allow the Saved Objects Client to use the request's auth scope, @@ -49,8 +52,6 @@ export function initSpacesRequestInterceptors(server) { if (isRequestingKibanaRoot && !urlContext) { try { - const { selectedSpace } = request.state; - const client = request.getSavedObjectsClient(); const { total, saved_objects: spaceObjects } = await client.find({ type: 'space' @@ -68,31 +69,20 @@ export function initSpacesRequestInterceptors(server) { urlContext } = space.attributes; - let destination; - if (urlContext) { - destination = `${basePath}/s/${urlContext}${defaultRoute}`; - } else { - destination = `${basePath}${defaultRoute}`; - } - + const destination = addSpaceUrlContext(basePath, urlContext, defaultRoute); + setSelectedSpace(request, reply, urlContext); return reply.redirect(destination); } if (total > 0) { - const preferredSpace = spaceObjects.find(so => so.attributes.urlContext === selectedSpace); + // If the user's previously selected space is still available, then send them there automatically if (preferredSpace) { - let destination; - if (preferredSpace.attributes.urlContext) { - destination = `${basePath}/s/${preferredSpace.attributes.urlContext}${defaultRoute}`; - } else { - destination = `${basePath}${defaultRoute}`; - } - + const destination = addSpaceUrlContext(basePath, preferredSpace.attributes.urlContext, defaultRoute); return reply.redirect(destination); } - // render spaces selector instead of home page + // otherwise, render spaces selector instead of home page const app = server.getHiddenUiAppById('space_selector'); return reply.renderApp(app, { spaces: spaceObjects.map(so => ({ ...so.attributes, id: so.id })) @@ -104,12 +94,8 @@ export function initSpacesRequestInterceptors(server) { } } - if (typeof urlContext === 'string' && request.state.selectedSpace !== urlContext) { - console.log('setting selectedSpace', urlContext); - reply.state('selectedSpace', urlContext); - } + setSelectedSpace(request, reply, urlContext); + return reply.continue(); }); - - } diff --git a/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js b/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js index 434ec530ba6f3..264f707bcf9c5 100644 --- a/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js +++ b/x-pack/plugins/spaces/server/lib/space_request_interceptors.test.js @@ -6,6 +6,13 @@ import sinon from 'sinon'; import { Server } from 'hapi'; import { initSpacesRequestInterceptors } from './space_request_interceptors'; +import { initSelectedSpaceState } from './selected_space_state'; + +const config = { + 'server.ssl.enabled': true, + 'server.basePath': '/', + 'xpack.spaces.rememberSelectedSpace': true, +}; describe('interceptors', () => { const sandbox = sinon.sandbox.create(); @@ -20,6 +27,12 @@ describe('interceptors', () => { server.connection({ port: 0 }); + initSelectedSpaceState(server, { + get: (key) => { + return config[key]; + } + }); + initSpacesRequestInterceptors(server); server.route({ diff --git a/x-pack/plugins/spaces/server/routes/api/v1/spaces.js b/x-pack/plugins/spaces/server/routes/api/v1/spaces.js index 64b893b93239d..6baed2e911947 100644 --- a/x-pack/plugins/spaces/server/routes/api/v1/spaces.js +++ b/x-pack/plugins/spaces/server/routes/api/v1/spaces.js @@ -12,6 +12,8 @@ import { spaceSchema } from '../../../lib/space_schema'; import { wrapError } from '../../../lib/errors'; import { isReservedSpace } from '../../../../common/is_reserved_space'; import { createDuplicateContextQuery } from '../../../lib/check_duplicate_context'; +import { setSelectedSpace } from '../../../lib/selected_space_state'; +import { addSpaceUrlContext } from '../../../../common'; export function initSpacesApi(server) { const routePreCheckLicenseFn = routePreCheckLicense(server); @@ -63,7 +65,7 @@ export function initSpacesApi(server) { }); spaces = result.saved_objects.map(convertSavedObjectToSpace); - } catch(e) { + } catch (e) { return reply(wrapError(e)); } @@ -156,7 +158,7 @@ export function initSpacesApi(server) { let result; try { result = await client.create('space', { ...space }, { id, overwrite }); - } catch(e) { + } catch (e) { return reply(wrapError(e)); } @@ -198,6 +200,30 @@ export function initSpacesApi(server) { } }); + server.route({ + method: 'PUT', + path: '/api/spaces/v1/space/{id}/select', + async handler(request, reply) { + const client = request.getSavedObjectsClient(); + + const id = request.params.id; + + try { + const existingSpace = await getSpaceById(client, id); + + setSelectedSpace(request, reply, existingSpace.urlContext); + const config = server.config(); + + return reply({ + location: addSpaceUrlContext(config.get('server.basePath'), existingSpace.urlContext) + }); + + } catch (e) { + return reply(wrapError(e)); + } + } + }); + async function getSpaceById(client, spaceId) { try { const existingSpace = await client.get('space', spaceId); From b8b190779fbebe6faafa8c30db3d21ae46a074b2 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 24 May 2018 14:06:41 -0400 Subject: [PATCH 3/6] Fix error on login/logout --- x-pack/plugins/spaces/index.js | 7 +++++++ x-pack/plugins/spaces/server/routes/api/v1/spaces.js | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/spaces/index.js b/x-pack/plugins/spaces/index.js index c7dc9801c4328..dd37e0a40d74e 100644 --- a/x-pack/plugins/spaces/index.js +++ b/x-pack/plugins/spaces/index.js @@ -49,6 +49,13 @@ export const spaces = (kibana) => new kibana.Plugin({ }; }, replaceInjectedVars: async function (vars, request) { + // A rather obtuse way of preventing the Kibana login/logout resources from trying to make these requests. + // This seems safer than excluding a couple of hard-coded paths. + const canReplace = request.path.startsWith('/app/'); + if (!canReplace) { + return vars; + } + try { vars.activeSpace = { valid: true, diff --git a/x-pack/plugins/spaces/server/routes/api/v1/spaces.js b/x-pack/plugins/spaces/server/routes/api/v1/spaces.js index 6baed2e911947..52dfcb255cd58 100644 --- a/x-pack/plugins/spaces/server/routes/api/v1/spaces.js +++ b/x-pack/plugins/spaces/server/routes/api/v1/spaces.js @@ -227,7 +227,6 @@ export function initSpacesApi(server) { async function getSpaceById(client, spaceId) { try { const existingSpace = await client.get('space', spaceId); - console.log(existingSpace); return { id: existingSpace.id, ...existingSpace.attributes From c60ce5bce9c54f11ca64c513f055b7ac27d8a878 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 24 May 2018 14:09:47 -0400 Subject: [PATCH 4/6] rename NavControlModal to NavControlPopover --- .../spaces/public/views/nav_control/nav_control.js | 4 ++-- .../{nav_control_modal.js => nav_control_popover.js} | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) rename x-pack/plugins/spaces/public/views/nav_control/{nav_control_modal.js => nav_control_popover.js} (93%) diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control.js index 6c686a4a30064..0f29cfea606c9 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control.js @@ -13,7 +13,7 @@ import 'plugins/spaces/views/nav_control/nav_control.less'; import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; -import { NavControlModal } from 'plugins/spaces/views/nav_control/nav_control_modal'; +import { NavControlPopover } from 'plugins/spaces/views/nav_control/nav_control_popover'; chromeNavControlsRegistry.register(constant({ name: 'spaces', @@ -28,7 +28,7 @@ module.controller('spacesNavController', ($scope, $http, chrome, globalNavState, const spacesManager = new SpacesManager($http, chrome); - render(, domNode); + render(, domNode); // unmount react on controller destroy $scope.$on('$destroy', () => { diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js similarity index 93% rename from x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js rename to x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js index d5ec849e75d0f..cfe9f071f1d31 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control_modal.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js @@ -13,9 +13,7 @@ import { import { Notifier } from 'ui/notify'; import { SpacesContextMenu } from './components/spaces_context_menu'; -export class NavControlModal extends Component { - buttonRef; - +export class NavControlPopover extends Component { state = { isOpen: false, loading: false, @@ -95,7 +93,7 @@ export class NavControlModal extends Component { getButton = (linkIcon, linkTitle) => { return ( -
{ this.buttonRef = me; }} onClick={this.expandGlobalNav}> +
{linkIcon}
{linkTitle}
@@ -133,7 +131,7 @@ export class NavControlModal extends Component { } } -NavControlModal.propTypes = { +NavControlPopover.propTypes = { activeSpace: PropTypes.object, spacesManager: PropTypes.object.isRequired, globalNavState: PropTypes.object.isRequired, From c0d151007c06fe3f37e69317075152f2dbbe8a92 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 24 May 2018 14:16:40 -0400 Subject: [PATCH 5/6] remove Manage button for this PR --- .../spaces/public/views/nav_control/nav_control_popover.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js b/x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js index cfe9f071f1d31..6e9aed4655731 100644 --- a/x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js +++ b/x-pack/plugins/spaces/public/views/nav_control/nav_control_popover.js @@ -23,7 +23,6 @@ export class NavControlPopover extends Component { notifier = new Notifier(`Spaces`); async loadSpaces() { - console.log('loading spaces'); const { spacesManager } = this.props; @@ -62,7 +61,7 @@ export class NavControlPopover extends Component { button={this.getActiveSpaceButton()} closePopover={this.closePortal} > - + ); } From 1fcb85af5d00df64a97ce85afe19af2a825b9bb7 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 24 May 2018 14:45:02 -0400 Subject: [PATCH 6/6] additional tests --- .../spaces/common/spaces_url_parser.test.js | 2 +- .../spaces_context_menu.test.js.snap | 198 ++++++++++++++++++ .../components/spaces_context_menu.test.js | 49 +++++ .../spaces/server/lib/selected_space_state.js | 2 +- .../server/routes/api/v1/spaces.test.js | 32 ++- 5 files changed, 279 insertions(+), 4 deletions(-) create mode 100644 x-pack/plugins/spaces/public/views/nav_control/components/__snapshots__/spaces_context_menu.test.js.snap create mode 100644 x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.test.js diff --git a/x-pack/plugins/spaces/common/spaces_url_parser.test.js b/x-pack/plugins/spaces/common/spaces_url_parser.test.js index 0b2c530a620c3..33a5bf1f8bd70 100644 --- a/x-pack/plugins/spaces/common/spaces_url_parser.test.js +++ b/x-pack/plugins/spaces/common/spaces_url_parser.test.js @@ -38,7 +38,7 @@ describe('getSpaceUrlContext', () => { test('it handles base url without a space url context', () => { const basePath = `/this/is/a/crazy/path/s`; - expect(getSpaceUrlContext(basePath)).toEqual(null); + expect(getSpaceUrlContext(basePath)).toEqual(''); }); }); diff --git a/x-pack/plugins/spaces/public/views/nav_control/components/__snapshots__/spaces_context_menu.test.js.snap b/x-pack/plugins/spaces/public/views/nav_control/components/__snapshots__/spaces_context_menu.test.js.snap new file mode 100644 index 0000000000000..283ee03f5682f --- /dev/null +++ b/x-pack/plugins/spaces/public/views/nav_control/components/__snapshots__/spaces_context_menu.test.js.snap @@ -0,0 +1,198 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`it renders the provided spaces 1`] = ` + + +
+ +
+ +
+

+ Select a Space +

+
+ +
+ + +
+ + a space + , + + b space + , + + Default Space + , + ] + } + watchedItemProps={ + Array [ + "data-id", + ] + } + > +
+
+ + + + + + + + + +
+
+ +
+ +
+ + +`; diff --git a/x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.test.js b/x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.test.js new file mode 100644 index 0000000000000..65408c01a1b01 --- /dev/null +++ b/x-pack/plugins/spaces/public/views/nav_control/components/spaces_context_menu.test.js @@ -0,0 +1,49 @@ +/* + * 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 React from 'react'; +import { shallow, mount } from 'enzyme'; +import { SpacesContextMenu } from './spaces_context_menu'; +import { EuiContextMenuItem } from '@elastic/eui'; + +const spaces = [{ + id: 'a-space', + name: 'a space', + urlContext: 'a-space', +}, { + id: 'b-space', + name: 'b space', + urlContext: 'b-space', +}, { + id: 'default', + name: 'Default Space', + urlContext: '', + _reserved: true +}]; + +test('it renders without blowing up', () => { + shallow(); +}); + +test('it renders the provided spaces', () => { + const wrapper = mount(); + expect(wrapper.find(EuiContextMenuItem)).toHaveLength(spaces.length); + + expect(wrapper).toMatchSnapshot(); +}); + +test('it calls the click handler when a space is clicked', () => { + const clickHandler = jest.fn(); + const wrapper = mount(); + + expect(clickHandler).toHaveBeenCalledTimes(0); + + wrapper.find('button[data-id="default"]').simulate('click'); + + expect(clickHandler).toHaveBeenCalledTimes(1); + const [calledWithSpace] = clickHandler.mock.calls[0]; + expect(calledWithSpace).toEqual(spaces[2]); +}); diff --git a/x-pack/plugins/spaces/server/lib/selected_space_state.js b/x-pack/plugins/spaces/server/lib/selected_space_state.js index 7a7c8abbff992..97c2b5ee1aa5e 100644 --- a/x-pack/plugins/spaces/server/lib/selected_space_state.js +++ b/x-pack/plugins/spaces/server/lib/selected_space_state.js @@ -18,7 +18,7 @@ export function initSelectedSpaceState(server, config) { ttl: SELECTED_SPACE_COOKIE_TTL_MILLIS, isHttpOnly: true, isSecure: config.get('server.ssl.enabled'), - path: config.get('server.basePath'), + path: config.get('server.basePath') || null, }); } diff --git a/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js b/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js index 2318b4cd24319..3b32d6b63f327 100644 --- a/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js +++ b/x-pack/plugins/spaces/server/routes/api/v1/spaces.test.js @@ -6,6 +6,7 @@ import { Server } from 'hapi'; import { initSpacesApi } from './spaces'; +import { initSelectedSpaceState } from '../../../lib/selected_space_state'; jest.mock('../../../lib/route_pre_check_license', () => { return { @@ -17,7 +18,7 @@ jest.mock('../../../../../../server/lib/get_client_shield', () => { return { getClient: () => { return { - callWithInternalUser: jest.fn(() => {}) + callWithInternalUser: jest.fn(() => { }) }; } }; @@ -48,6 +49,12 @@ describe('Spaces API', () => { const teardowns = []; let request; + const config = { + 'server.ssl.enabled': true, + 'server.basePath': '', + 'xpack.spaces.rememberSelectedSpace': true + }; + beforeEach(() => { request = async (method, path, setupFn = () => { }) => { @@ -59,10 +66,11 @@ describe('Spaces API', () => { server.decorate('server', 'config', jest.fn(() => { return { - get: () => '' + get: (key) => config[key] }; })); + initSelectedSpaceState(server, server.config()); initSpacesApi(server); server.decorate('request', 'getBasePath', jest.fn()); @@ -148,4 +156,24 @@ describe('Spaces API', () => { message: "This Space cannot be deleted because it is reserved." }); }); + + test('PUT space/{id}/select should set a cookie with the new Space, and respond with a new location', async () => { + const response = await request('PUT', '/api/spaces/v1/space/a-space/select'); + + const { + statusCode, + headers, + payload + } = response; + + expect(statusCode).toEqual(200); + + expect(headers).toHaveProperty('set-cookie'); + expect(headers['set-cookie']).toHaveLength(1); + const [nameValuePair] = headers['set-cookie'][0].split(';').map(a => a.trim()); + expect(nameValuePair).toEqual('selectedSpace=a-space'); + + const result = JSON.parse(payload); + expect(result.location).toEqual('/s/a-space'); + }); });