From 19dc66900d8661d9ce1c81aed70ca1d16aaf1652 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Tue, 20 Oct 2020 06:45:35 +0200 Subject: [PATCH] Split the `trusted` setting into two new ones: - `allow_user_trust_override` - `clear_cache_on_logout` The `persistent_store` setting can now also be set to `sessionStorage` The `trusted` settings was in effect playing the role of two separate settings and implicitly affecting a third ('persistent_store'). By breaking it up, we make things more explicit and allow for new configurations. For example, clearing the cache on logout, while using some kind of persistent store. --- CHANGES.md | 4 ++ docs/source/configuration.rst | 120 ++++++++++++++++++---------------- spec/login.js | 63 ++++++++---------- spec/mock.js | 12 ++-- src/converse-controlbox.js | 22 ++----- src/converse-omemo.js | 8 ++- src/headless/connection.js | 1 - src/headless/converse-core.js | 33 ++++++---- src/headless/utils/stanza.js | 2 +- src/templates/login_panel.js | 15 ++--- 10 files changed, 147 insertions(+), 133 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 7e915a343..1632751a8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -36,6 +36,10 @@ Soon we'll deprecate the latter, so prepare now. - #2220: fix rendering of emojis in case `use_system_emojis == false` (again). - #2092: fixes room list update loop when having the `locked_muc_domain` truthy or `'hidden'` - #2285: Rename config option `muc_hats_from_vcard` to [muc_hats](https://conversejs.org/docs/html/configuration.html#muc-hats). Now accepts a list instead of a boolean and allows for more flexible choices regarding user badges. +- The `trusted` configuration setting has been removed in favor of two new settings: + [allow_user_trust_override](https://conversejs.org/docs/html/configuration.html#allow-user-trust-override) + [clear_cache_on_logout](https://conversejs.org/docs/html/configuration.html#clear-cache-on-logout) +- The `persistent_store` setting can now also be set to `sessionStorage` - The `api.archive.query` method no longer accepts an RSM instance as argument. - The plugin `converse-uniview` has been removed and its functionality merged into `converse-chatboxviews` - Removed the mockups from the project. Recommended to use tests instead. diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index ffd2593c6..2abfb64f0 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -35,8 +35,8 @@ login ~~~~~ The default means is ``login``, which means that the user either logs in manually with their -username and password, or automatically if used together with ``auto_login=true`` -and ``jid`` and ``password`` values. See `auto_login`_. +username and password, or automatically if used together with `auto_login`_ set to ``true`` +and ``jid`` and ``password`` values. external ~~~~~~~~ @@ -83,7 +83,7 @@ A JID (jabber ID), SID (session ID) and RID (Request ID). Converse needs these tokens in order to attach to that same session. -In addition to setting ``authentication`` to ``prebind``, you'll also need to +In addition to setting `authentication`_ to ``prebind``, you'll also need to set the `prebind_url`_ and `bosh-service-url`_. Here's an example of Converse being initialized with these options: @@ -243,6 +243,35 @@ Support for `XEP-0077: In band registration `_ @@ -528,6 +557,23 @@ A more modern alternative to BOSH is to use `websockets `_ -(``localStorage`` or ``sessionStorage``) used by Converse to cache user data. - -If ``trusted`` is set to ``false``, then ``sessionStorage`` is used instead of -``localStorage``. - -The main difference between the two is that ``sessionStorage`` only persists while -the current tab or window containing a Converse instance is open. As soon as -it's closed, the data is cleared (as long as there aren't any other tabs with -the same domain open). - -Data in ``localStorage`` on the other hand is kept indefinitely. - -The data that is cached includes your sent and received messages, which chats you had -open, what features the XMPP server supports and what your online status was. - -Clearing the cache makes Converse much slower when the user logs -in again, because all data needs to be fetch anew. - -If ``trusted`` is set to ``on`` or ``off`` the "This is a trusted device" -checkbox in the login form will not appear at all and cannot be changed by the user. -``on`` means to trust the device as stated above and use ``localStorage``. ``off`` -means to not trust the device (cache is cleared when the user logs out) and to use -``sessionStorage``. - time_format ----------- diff --git a/spec/login.js b/spec/login.js index 3ee07b9e1..2478a617c 100644 --- a/spec/login.js +++ b/spec/login.js @@ -1,4 +1,4 @@ -/*global mock */ +/*global mock, converse */ const u = converse.env.utils; @@ -11,8 +11,8 @@ describe("The Login Form", function () { allow_registration: false }, async function (done, _converse) { - mock.openControlBox(_converse); const cbview = await u.waitUntil(() => _converse.chatboxviews.get('controlbox')); + mock.toggleControlBox(); const checkboxes = cbview.el.querySelectorAll('input[type="checkbox"]'); expect(checkboxes.length).toBe(1); @@ -24,17 +24,16 @@ describe("The Login Form", function () { cbview.el.querySelector('input[name="jid"]').value = 'romeo@montague.lit'; cbview.el.querySelector('input[name="password"]').value = 'secret'; - spyOn(cbview.loginpanel, 'connect'); - cbview.delegateEvents(); - - expect(_converse.config.get('storage')).toBe('persistent'); + expect(_converse.config.get('trusted')).toBe(true); + expect(_converse.getDefaultStore()).toBe('persistent'); cbview.el.querySelector('input[type="submit"]').click(); - expect(_converse.config.get('storage')).toBe('persistent'); - expect(cbview.loginpanel.connect).toHaveBeenCalled(); + expect(_converse.config.get('trusted')).toBe(true); + expect(_converse.getDefaultStore()).toBe('persistent'); checkbox.click(); cbview.el.querySelector('input[type="submit"]').click(); - expect(_converse.config.get('storage')).toBe('session'); + expect(_converse.config.get('trusted')).toBe(false); + expect(_converse.getDefaultStore()).toBe('session'); done(); })); @@ -42,36 +41,32 @@ describe("The Login Form", function () { mock.initConverse( ['chatBoxesInitialized'], { auto_login: false, - trusted: false, + allow_user_trust_override: 'off', allow_registration: false }, - function (done, _converse) { + async function (done, _converse) { - u.waitUntil(() => _converse.chatboxviews.get('controlbox')) - .then(() => { - var cbview = _converse.chatboxviews.get('controlbox'); - mock.openControlBox(_converse); - const checkboxes = cbview.el.querySelectorAll('input[type="checkbox"]'); - expect(checkboxes.length).toBe(1); + await u.waitUntil(() => _converse.chatboxviews.get('controlbox')) + const cbview = _converse.chatboxviews.get('controlbox'); + mock.toggleControlBox(); + const checkboxes = cbview.el.querySelectorAll('input[type="checkbox"]'); + expect(checkboxes.length).toBe(1); - const checkbox = checkboxes[0]; - const label = cbview.el.querySelector(`label[for="${checkbox.getAttribute('id')}"]`); - expect(label.textContent).toBe('This is a trusted device'); - expect(checkbox.checked).toBe(false); + const checkbox = checkboxes[0]; + const label = cbview.el.querySelector(`label[for="${checkbox.getAttribute('id')}"]`); + expect(label.textContent).toBe('This is a trusted device'); + expect(checkbox.checked).toBe(false); - cbview.el.querySelector('input[name="jid"]').value = 'romeo@montague.lit'; - cbview.el.querySelector('input[name="password"]').value = 'secret'; + cbview.el.querySelector('input[name="jid"]').value = 'romeo@montague.lit'; + cbview.el.querySelector('input[name="password"]').value = 'secret'; - spyOn(cbview.loginpanel, 'connect'); + cbview.el.querySelector('input[type="submit"]').click(); + expect(_converse.config.get('trusted')).toBe(false); + expect(_converse.getDefaultStore()).toBe('session'); - expect(_converse.config.get('storage')).toBe('session'); - cbview.el.querySelector('input[type="submit"]').click(); - expect(_converse.config.get('storage')).toBe('session'); - expect(cbview.loginpanel.connect).toHaveBeenCalled(); - - checkbox.click(); - cbview.el.querySelector('input[type="submit"]').click(); - expect(_converse.config.get('storage')).toBe('persistent'); - done(); - }); + checkbox.click(); + cbview.el.querySelector('input[type="submit"]').click(); + expect(_converse.config.get('trusted')).toBe(true); + expect(_converse.getDefaultStore()).toBe('persistent'); + done(); })); }); diff --git a/spec/mock.js b/spec/mock.js index 55714af75..5ac9da775 100644 --- a/spec/mock.js +++ b/spec/mock.js @@ -75,16 +75,20 @@ window.addEventListener('converse-loaded', () => { return Promise.all(_converse.chatboxviews.map(view => view.close())); }; - mock.openControlBox = async function (_converse) { - const model = await _converse.api.controlbox.open(); - await u.waitUntil(() => model.get('connected')); - var toggle = document.querySelector(".toggle-controlbox"); + mock.toggleControlBox = function () { + const toggle = document.querySelector(".toggle-controlbox"); if (!u.isVisible(document.querySelector("#controlbox"))) { if (!u.isVisible(toggle)) { u.removeClass('hidden', toggle); } toggle.click(); } + } + + mock.openControlBox = async function (_converse) { + const model = await _converse.api.controlbox.open(); + await u.waitUntil(() => model.get('connected')); + mock.toggleControlBox(); return this; }; diff --git a/src/converse-controlbox.js b/src/converse-controlbox.js index a1c2d9b58..34ecad3f4 100644 --- a/src/converse-controlbox.js +++ b/src/converse-controlbox.js @@ -102,6 +102,7 @@ converse.plugins.add('converse-controlbox', { */ api.settings.extend({ allow_logout: true, + allow_user_trust_override: true, default_domain: undefined, locked_domain: undefined, show_controlbox_by_default: false, @@ -378,7 +379,7 @@ converse.plugins.add('converse-controlbox', { 'conn_feedback_message': _converse.connfeedback.get('message'), 'placeholder_username': (api.settings.get('locked_domain') || api.settings.get('default_domain')) && __('Username') || __('user@domain'), - 'show_trust_checkbox': _converse.trusted !== 'on' && _converse.trusted !== 'off' + 'show_trust_checkbox': api.settings.get('allow_user_trust_override') }) ); }, @@ -407,9 +408,11 @@ converse.plugins.add('converse-controlbox', { return true; }, + /** + * Authenticate the user based on a form submission event. + * @param { Event } ev + */ authenticate (ev) { - /* Authenticate the user based on a form submission event. - */ if (ev && ev.preventDefault) { ev.preventDefault(); } if (api.settings.get("authentication") === _converse.ANONYMOUS) { return this.connect(_converse.jid, null); @@ -417,18 +420,7 @@ converse.plugins.add('converse-controlbox', { if (!this.validate()) { return; } const form_data = new FormData(ev.target); - - if (_converse.trusted === 'on' || _converse.trusted === 'off') { - _converse.config.save({ - 'trusted': _converse.trusted === 'on', - 'storage': _converse.trusted === 'on' ? 'persistent' : 'session' - }); - } else { - _converse.config.save({ - 'trusted': form_data.get('trusted') && true || false, - 'storage': form_data.get('trusted') ? 'persistent' : 'session' - }); - } + _converse.config.save({ 'trusted': form_data.get('trusted') && true || false }); let jid = form_data.get('jid'); if (api.settings.get('locked_domain')) { diff --git a/src/converse-omemo.js b/src/converse-omemo.js index 5c2634e94..b735df967 100644 --- a/src/converse-omemo.js +++ b/src/converse-omemo.js @@ -405,7 +405,8 @@ async function fetchOwnDevices () { } async function initOMEMO () { - if (!_converse.config.get('trusted')) { + if (!_converse.config.get('trusted') || api.settings.get('clear_cache_on_logout')) { + log.warn("Not initializing OMEMO, since this browser is not trusted or clear_cache_on_logout is set to true"); return; } _converse.devicelists = new _converse.DeviceLists(); @@ -513,7 +514,9 @@ function getOMEMOToolbarButton (toolbar_el, buttons) { converse.plugins.add('converse-omemo', { enabled (_converse) { - return window.libsignal && !_converse.api.settings.get("blacklisted_plugins").includes('converse-omemo') && _converse.config.get('trusted'); + return window.libsignal && + !_converse.api.settings.get("blacklisted_plugins").includes('converse-omemo') && + (_converse.config.get('trusted') || !api.settings.get('clear_cache_on_logout')); }, dependencies: ["converse-chatview", "converse-pubsub", "converse-profile"], @@ -1358,4 +1361,3 @@ converse.plugins.add('converse-omemo', { }); } }); - diff --git a/src/headless/connection.js b/src/headless/connection.js index 48e284b2a..8c1b84d2a 100644 --- a/src/headless/connection.js +++ b/src/headless/connection.js @@ -404,4 +404,3 @@ export class MockConnection extends Connection { } } } - diff --git a/src/headless/converse-core.js b/src/headless/converse-core.js index 81ee42d40..60762ceff 100644 --- a/src/headless/converse-core.js +++ b/src/headless/converse-core.js @@ -95,13 +95,14 @@ const DEFAULT_SETTINGS = { auto_login: false, // Currently only used in connection with anonymous login auto_reconnect: true, blacklisted_plugins: [], + clear_cache_on_logout: false, connection_options: {}, credentials_url: null, // URL from where login credentials can be fetched discover_connection_methods: true, geouri_regex: /https\:\/\/www.openstreetmap.org\/.*#map=[0-9]+\/([\-0-9.]+)\/([\-0-9.]+)\S*/g, geouri_replacement: 'https://www.openstreetmap.org/?mlat=$1&mlon=$2#map=18/$1/$2', - idle_presence_timeout: 300, // Seconds after which an idle presence is sent i18n: 'en', + idle_presence_timeout: 300, // Seconds after which an idle presence is sent jid: undefined, keepalive: true, loglevel: 'info', @@ -118,7 +119,6 @@ const DEFAULT_SETTINGS = { sid: undefined, singleton: false, strict_plugin_dependencies: false, - trusted: true, view_mode: 'overlayed', // Choices are 'overlayed', 'fullscreen', 'mobile' websocket_url: undefined, whitelisted_plugins: [] @@ -585,7 +585,7 @@ export const api = _converse.api = { /** * Get the value of a particular user setting. * @method _converse.api.user.settings.get - * @param {String} key - hello world + * @param {String} key - The setting name * @param {*} fallback - An optional fallback value if the user setting is undefined * @returns {Promise} Promise which resolves with the value of the particular configuration setting. * @example _converse.api.user.settings.get("foo"); @@ -688,6 +688,7 @@ export const api = _converse.api = { return _converse[key]; } }, + /** * Set one or many configuration settings. * @@ -973,7 +974,7 @@ async function initSessionStorage () { function initPersistentStorage () { - if (_converse.config.get('storage') !== 'persistent') { + if (api.settings.get('persistent_store') === 'sessionStorage') { return; } const config = { @@ -991,8 +992,18 @@ function initPersistentStorage () { } +_converse.getDefaultStore = function () { + if (_converse.config.get('trusted')) { + const is_non_persistent = api.settings.get('persistent_store') === 'sessionStorage'; + return is_non_persistent ? 'session': 'persistent'; + } else { + return 'session'; + } +} + + function createStore (id, storage) { - const s = _converse.storage[storage ? storage : _converse.config.get('storage')]; + const s = _converse.storage[storage || _converse.getDefaultStore()]; return new Storage(id, s); } @@ -1049,11 +1060,7 @@ function initClientConfig () { * user sessions. */ const id = 'converse.client-config'; - _converse.config = new Model({ - 'id': id, - 'trusted': _converse.api.settings.get("trusted") && true || false, - 'storage': _converse.api.settings.get("trusted") ? 'persistent' : 'session' - }); + _converse.config = new Model({ id, 'trusted': true }); _converse.config.browserStorage = createStore(id, "session"); _converse.config.fetch(); /** @@ -1141,7 +1148,11 @@ function connect (credentials) { } -_converse.shouldClearCache = () => (!_converse.config.get('trusted') || _converse.isTestEnv()); +_converse.shouldClearCache = () => ( + !_converse.config.get('trusted') || + api.settings.get('clear_cache_on_logout') || + _converse.isTestEnv() +); export function clearSession () { diff --git a/src/headless/utils/stanza.js b/src/headless/utils/stanza.js index 79f9dfa24..fa642126a 100644 --- a/src/headless/utils/stanza.js +++ b/src/headless/utils/stanza.js @@ -51,7 +51,7 @@ function getCorrectionAttributes (stanza, original_stanza) { function getEncryptionAttributes (stanza, _converse) { const encrypted = sizzle(`encrypted[xmlns="${Strophe.NS.OMEMO}"]`, stanza).pop(); const attrs = { 'is_encrypted': !!encrypted }; - if (!encrypted || !_converse.config.get('trusted')) { + if (!encrypted || api.settings.get('clear_cache_on_logout')) { return attrs; } const header = encrypted.querySelector('header'); diff --git a/src/templates/login_panel.js b/src/templates/login_panel.js index a4ea6848c..3f1f9f6eb 100644 --- a/src/templates/login_panel.js +++ b/src/templates/login_panel.js @@ -1,10 +1,10 @@ import tpl_spinner from './spinner.js'; import { __ } from '../i18n'; -import { api } from "@converse/headless/converse-core"; +import { _converse, api } from "@converse/headless/converse-core"; import { html } from "lit-html"; -const trust_checkbox = (o) => { +const trust_checkbox = (checked) => { const i18n_hint_trusted = __( 'To improve performance, we cache your data in this browser. '+ 'Uncheck this box if this is a public computer or if you want your data to be deleted when you log out. '+ @@ -13,7 +13,7 @@ const trust_checkbox = (o) => { const i18n_trusted = __('This is a trusted device'); return html` ${ (o.authentication !== o.EXTERNAL) ? password_input() : '' } - ${ o.show_trust_checkbox ? trust_checkbox(o) : '' } + ${ o.show_trust_checkbox ? trust_checkbox(o.show_trust_checkbox === 'off' ? false : true) : '' }
- ${ show_register_link(o) ? register_link(o) : '' } + ${ show_register_link() ? register_link(o) : '' } `; } @@ -93,6 +92,6 @@ export default (o) => html` - ${ (o._converse.CONNECTION_STATUS[o.connection_status] === 'CONNECTING') ? tpl_spinner({'classes': 'hor_centered'}) : form_fields(o) } + ${ (_converse.CONNECTION_STATUS[o.connection_status] === 'CONNECTING') ? tpl_spinner({'classes': 'hor_centered'}) : form_fields(o) } `;