From 939f6d07984cd51241357a5e61bf76bd46179fcf Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 13 Jun 2017 12:46:49 +0100 Subject: [PATCH 1/2] Factor createMatrixClient out from MatrixClientPeg ... so that it can be used elsewhere. --- src/MatrixClientPeg.js | 21 ++----------- src/utils/createMatrixClient.js | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 19 deletions(-) create mode 100644 src/utils/createMatrixClient.js diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 94e55a8d8a..47370e2142 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -16,12 +16,10 @@ limitations under the License. 'use strict'; -import Matrix from 'matrix-js-sdk'; import utils from 'matrix-js-sdk/lib/utils'; import EventTimeline from 'matrix-js-sdk/lib/models/event-timeline'; import EventTimelineSet from 'matrix-js-sdk/lib/models/event-timeline-set'; - -const localStorage = window.localStorage; +import createMatrixClient from './utils/createMatrixClient'; interface MatrixClientCreds { homeserverUrl: string, @@ -129,22 +127,7 @@ class MatrixClientPeg { timelineSupport: true, }; - if (localStorage) { - opts.sessionStore = new Matrix.WebStorageSessionStore(localStorage); - } - if (window.indexedDB && localStorage) { - // FIXME: bodge to remove old database. Remove this after a few weeks. - window.indexedDB.deleteDatabase("matrix-js-sdk:default"); - - opts.store = new Matrix.IndexedDBStore({ - indexedDB: window.indexedDB, - dbName: "riot-web-sync", - localStorage: localStorage, - workerScript: this.indexedDbWorkerScript, - }); - } - - this.matrixClient = Matrix.createClient(opts); + this.matrixClient = createMatrixClient(opts); // we're going to add eventlisteners for each matrix event tile, so the // potential number of event listeners is quite high. diff --git a/src/utils/createMatrixClient.js b/src/utils/createMatrixClient.js new file mode 100644 index 0000000000..5effd63f2a --- /dev/null +++ b/src/utils/createMatrixClient.js @@ -0,0 +1,55 @@ +/* +Copyright 2017 Vector Creations Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import Matrix from 'matrix-js-sdk'; + +const localStorage = window.localStorage; + +/** + * Create a new matrix client, with the persistent stores set up appropriately + * (using localstorage/indexeddb, etc) + * + * @param {Object} opts options to pass to Matrix.createClient. This will be + * extended with `sessionStore` and `store` members. + * + * @param {string} indexedDbWorkerScript Optional URL for a web worker script + * for IndexedDB store operations. If not given, indexeddb ops are done on + * the main thread. + * + * @returns {MatrixClient} the newly-created MatrixClient + */ +export default function createMatrixClient(opts, indexedDbWorkerScript) { + const storeOpts = {}; + + if (localStorage) { + storeOpts.sessionStore = new Matrix.WebStorageSessionStore(localStorage); + } + if (window.indexedDB && localStorage) { + // FIXME: bodge to remove old database. Remove this after a few weeks. + window.indexedDB.deleteDatabase("matrix-js-sdk:default"); + + storeOpts.store = new Matrix.IndexedDBStore({ + indexedDB: window.indexedDB, + dbName: "riot-web-sync", + localStorage: localStorage, + workerScript: indexedDbWorkerScript, + }); + } + + opts = Object.assign(storeOpts, opts); + + return Matrix.createClient(opts); +} From 68e1a7be7424d2d43caf71f5e5c05d52b9e7c07a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 31 May 2017 17:28:46 +0100 Subject: [PATCH 2/2] Clear persistent storage on login and logout Make sure that we don't end up with sensitive data sitting around in the stores from a previous session. --- src/Lifecycle.js | 107 +++++++++++++++--------- src/components/structures/MatrixChat.js | 2 + 2 files changed, 71 insertions(+), 38 deletions(-) diff --git a/src/Lifecycle.js b/src/Lifecycle.js index 54014a0166..9b806ba8b7 100644 --- a/src/Lifecycle.js +++ b/src/Lifecycle.js @@ -19,6 +19,7 @@ import q from 'q'; import Matrix from 'matrix-js-sdk'; import MatrixClientPeg from './MatrixClientPeg'; +import createMatrixClient from './utils/createMatrixClient'; import Analytics from './Analytics'; import Notifier from './Notifier'; import UserActivity from './UserActivity'; @@ -48,7 +49,7 @@ import { _t } from './languageHandler'; * * 4. it attempts to auto-register as a guest user. * - * If any of steps 1-4 are successful, it will call {setLoggedIn}, which in + * If any of steps 1-4 are successful, it will call {_doSetLoggedIn}, which in * turn will raise on_logged_in and will_start_client events. * * @param {object} opts @@ -105,14 +106,13 @@ export function loadSession(opts) { fragmentQueryParams.guest_access_token ) { console.log("Using guest access credentials"); - setLoggedIn({ + return _doSetLoggedIn({ userId: fragmentQueryParams.guest_user_id, accessToken: fragmentQueryParams.guest_access_token, homeserverUrl: guestHsUrl, identityServerUrl: guestIsUrl, guest: true, - }); - return q(); + }, true); } return _restoreFromLocalStorage().then((success) => { @@ -141,14 +141,14 @@ function _loginWithToken(queryParams, defaultDeviceDisplayName) { }, ).then(function(data) { console.log("Logged in with token"); - setLoggedIn({ + return _doSetLoggedIn({ userId: data.user_id, deviceId: data.device_id, accessToken: data.access_token, homeserverUrl: queryParams.homeserver, identityServerUrl: queryParams.identityServer, guest: false, - }); + }, true); }, (err) => { console.error("Failed to log in with login token: " + err + " " + err.data); @@ -172,14 +172,14 @@ function _registerAsGuest(hsUrl, isUrl, defaultDeviceDisplayName) { }, }).then((creds) => { console.log("Registered as guest: %s", creds.user_id); - setLoggedIn({ + return _doSetLoggedIn({ userId: creds.user_id, deviceId: creds.device_id, accessToken: creds.access_token, homeserverUrl: hsUrl, identityServerUrl: isUrl, guest: true, - }); + }, true); }, (err) => { console.error("Failed to register as guest: " + err + " " + err.data); }); @@ -216,15 +216,14 @@ function _restoreFromLocalStorage() { if (accessToken && userId && hsUrl) { console.log("Restoring session for %s", userId); try { - setLoggedIn({ + return _doSetLoggedIn({ userId: userId, deviceId: deviceId, accessToken: accessToken, homeserverUrl: hsUrl, identityServerUrl: isUrl, guest: isGuest, - }); - return q(true); + }, false).then(() => true); } catch (e) { return _handleRestoreFailure(e); } @@ -245,7 +244,7 @@ function _handleRestoreFailure(e) { + ' This is a once off; sorry for the inconvenience.', ); - _clearLocalStorage(); + _clearStorage(); return q.reject(new Error( _t('Unable to restore previous session') + ': ' + msg, @@ -266,7 +265,7 @@ function _handleRestoreFailure(e) { return def.promise.then((success) => { if (success) { // user clicked continue. - _clearLocalStorage(); + _clearStorage(); return false; } @@ -281,13 +280,32 @@ export function initRtsClient(url) { } /** - * Transitions to a logged-in state using the given credentials + * Transitions to a logged-in state using the given credentials. + * + * Starts the matrix client and all other react-sdk services that + * listen for events while a session is logged in. + * + * Also stops the old MatrixClient and clears old credentials/etc out of + * storage before starting the new client. + * * @param {MatrixClientCreds} credentials The credentials to use */ export function setLoggedIn(credentials) { - credentials.guest = Boolean(credentials.guest); + stopMatrixClient(); + _doSetLoggedIn(credentials, true); +} - Analytics.setGuest(credentials.guest); +/** + * fires on_logging_in, optionally clears localstorage, persists new credentials + * to localstorage, starts the new client. + * + * @param {MatrixClientCreds} credentials + * @param {Boolean} clearStorage + * + * returns a Promise which resolves once the client has been started + */ +async function _doSetLoggedIn(credentials, clearStorage) { + credentials.guest = Boolean(credentials.guest); console.log( "setLoggedIn: mxid:", credentials.userId, @@ -295,12 +313,19 @@ export function setLoggedIn(credentials) { "guest:", credentials.guest, "hs:", credentials.homeserverUrl, ); + // This is dispatched to indicate that the user is still in the process of logging in // because `teamPromise` may take some time to resolve, breaking the assumption that // `setLoggedIn` takes an "instant" to complete, and dispatch `on_logged_in` a few ms // later than MatrixChat might assume. dis.dispatch({action: 'on_logging_in'}); + if (clearStorage) { + await _clearStorage(); + } + + Analytics.setGuest(credentials.guest); + // Resolves by default let teamPromise = Promise.resolve(null); @@ -349,9 +374,6 @@ export function setLoggedIn(credentials) { console.warn("No local storage available: can't persist session!"); } - // stop any running clients before we create a new one with these new credentials - stopMatrixClient(); - MatrixClientPeg.replaceUsingCreds(credentials); teamPromise.then((teamToken) => { @@ -400,7 +422,7 @@ export function logout() { * Starts the matrix client and all other react-sdk services that * listen for events while a session is logged in. */ -export function startMatrixClient() { +function startMatrixClient() { // dispatch this before starting the matrix client: it's used // to add listeners for the 'sync' event so otherwise we'd have // a race condition (and we need to dispatch synchronously for this @@ -416,34 +438,44 @@ export function startMatrixClient() { } /* - * Stops a running client and all related services, used after - * a session has been logged out / ended. + * Stops a running client and all related services, and clears persistent + * storage. Used after a session has been logged out. */ export function onLoggedOut() { - _clearLocalStorage(); stopMatrixClient(); + _clearStorage().done(); dis.dispatch({action: 'on_logged_out'}); } -function _clearLocalStorage() { +/** + * @returns {Promise} promise which resolves once the stores have been cleared + */ +function _clearStorage() { Analytics.logout(); - if (!window.localStorage) { - return; - } - const hsUrl = window.localStorage.getItem("mx_hs_url"); - const isUrl = window.localStorage.getItem("mx_is_url"); - window.localStorage.clear(); - // preserve our HS & IS URLs for convenience - // N.B. we cache them in hsUrl/isUrl and can't really inline them - // as getCurrentHsUrl() may call through to localStorage. - // NB. We do clear the device ID (as well as all the settings) - if (hsUrl) window.localStorage.setItem("mx_hs_url", hsUrl); - if (isUrl) window.localStorage.setItem("mx_is_url", isUrl); + if (window.localStorage) { + const hsUrl = window.localStorage.getItem("mx_hs_url"); + const isUrl = window.localStorage.getItem("mx_is_url"); + window.localStorage.clear(); + + // preserve our HS & IS URLs for convenience + // N.B. we cache them in hsUrl/isUrl and can't really inline them + // as getCurrentHsUrl() may call through to localStorage. + // NB. We do clear the device ID (as well as all the settings) + if (hsUrl) window.localStorage.setItem("mx_hs_url", hsUrl); + if (isUrl) window.localStorage.setItem("mx_is_url", isUrl); + } + + // create a temporary client to clear out the persistent stores. + const cli = createMatrixClient({ + // we'll never make any requests, so can pass a bogus HS URL + baseUrl: "", + }); + return cli.clearStores(); } /** - * Stop all the background processes related to the current client + * Stop all the background processes related to the current client. */ export function stopMatrixClient() { Notifier.stop(); @@ -454,7 +486,6 @@ export function stopMatrixClient() { if (cli) { cli.stopClient(); cli.removeAllListeners(); - cli.store.deleteAllData(); MatrixClientPeg.unset(); } } diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 1065fa9f9f..71a82d0920 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -1228,6 +1228,8 @@ module.exports = React.createClass({ onReturnToGuestClick: function() { // reanimate our guest login if (this.state.guestCreds) { + // TODO: this is probably a bit broken - we don't want to be + // clearing storage when we reanimate the guest creds. Lifecycle.setLoggedIn(this.state.guestCreds); this.setState({guestCreds: null}); }