浏览代码

Refactor the login form

Render the form based on `api.settings` instead of its own model.

When the login form is submitted, save the JID, password and connection
URL to `api.settings`.

Set the `service` on the Strophe connection object just before
connecting for the first time, otherwise a user supplied URL (via the
login form) is never used.

New API setting: show_connection_url_input
JC Brand 3 年之前
父节点
当前提交
fca275b7c9

+ 1 - 0
CHANGES.md

@@ -21,6 +21,7 @@
 - #2814: Links are mangled on open/copy
 - #2814: Links are mangled on open/copy
 - #2822: Singleton doesn't work in overlayed view mode
 - #2822: Singleton doesn't work in overlayed view mode
 
 
+- New config option [show_connection_url_input](https://conversejs.org/docs/html/configuration.html#show-connection-url-input)
 
 
 ## 9.0.0 (2021-11-26)
 ## 9.0.0 (2021-11-26)
 
 

+ 10 - 0
docs/source/configuration.rst

@@ -1969,6 +1969,16 @@ show_client_info
 Specifies whether the info icon is shown on the controlbox which when clicked opens an
 Specifies whether the info icon is shown on the controlbox which when clicked opens an
 "About" modal with more information about the version of Converse being used.
 "About" modal with more information about the version of Converse being used.
 
 
+show_connection_url_input
+-------------------------
+
+* Default: ``false``
+
+Determines whether the login form should show an input element where the user
+can enter the connection URL. If it's a websocket url, then upon form
+submission the `websocket_url`_ setting will be updated with this value, and if
+it's an HTTP URL then the `bosh_service_url`_ setting will be updated.
+
 show_controlbox_by_default
 show_controlbox_by_default
 --------------------------
 --------------------------
 
 

+ 6 - 11
src/headless/core.js

@@ -22,7 +22,7 @@ import { Strophe, $build, $iq, $msg, $pres } from 'strophe.js/src/strophe';
 import { TimeoutError } from '@converse/headless/shared/errors';
 import { TimeoutError } from '@converse/headless/shared/errors';
 import { getOpenPromise } from '@converse/openpromise';
 import { getOpenPromise } from '@converse/openpromise';
 import { html } from 'lit';
 import { html } from 'lit';
-import { initAppSettings, } from '@converse/headless/shared/settings/utils.js';
+import { initAppSettings } from '@converse/headless/shared/settings/utils.js';
 import { settings_api, user_settings_api } from '@converse/headless/shared/settings/api.js';
 import { settings_api, user_settings_api } from '@converse/headless/shared/settings/api.js';
 import { sprintf } from 'sprintf-js';
 import { sprintf } from 'sprintf-js';
 
 
@@ -32,11 +32,12 @@ export { i18n };
 import {
 import {
     attemptNonPreboundSession,
     attemptNonPreboundSession,
     cleanup,
     cleanup,
+    getConnectionServiceURL,
     initClientConfig,
     initClientConfig,
     initPlugins,
     initPlugins,
-    setUserJID,
     initSessionStorage,
     initSessionStorage,
-    registerGlobalEventHandlers
+    registerGlobalEventHandlers,
+    setUserJID,
 } from './utils/init.js';
 } from './utils/init.js';
 
 
 dayjs.extend(advancedFormat);
 dayjs.extend(advancedFormat);
@@ -265,7 +266,7 @@ export const api = _converse.api = {
 
 
             // See whether there is a BOSH session to re-attach to
             // See whether there is a BOSH session to re-attach to
             const bosh_plugin = _converse.pluggable.plugins['converse-bosh'];
             const bosh_plugin = _converse.pluggable.plugins['converse-bosh'];
-            if (bosh_plugin && bosh_plugin.enabled()) {
+            if (bosh_plugin?.enabled()) {
                 if (await _converse.restoreBOSHSession()) {
                 if (await _converse.restoreBOSHSession()) {
                     return;
                     return;
                 } else if (api.settings.get("authentication") === _converse.PREBIND && (!automatic || api.settings.get("auto_login"))) {
                 } else if (api.settings.get("authentication") === _converse.PREBIND && (!automatic || api.settings.get("auto_login"))) {
@@ -559,15 +560,9 @@ _converse.initConnection = function () {
         }
         }
     }
     }
 
 
-    let connection_url = '';
     const XMPPConnection = _converse.isTestEnv() ? MockConnection : Connection;
     const XMPPConnection = _converse.isTestEnv() ? MockConnection : Connection;
-    if (('WebSocket' in window || 'MozWebSocket' in window) && api.settings.get("websocket_url")) {
-        connection_url = api.settings.get('websocket_url');
-    } else if (api.settings.get('bosh_service_url')) {
-        connection_url = api.settings.get('bosh_service_url');
-    }
     _converse.connection = new XMPPConnection(
     _converse.connection = new XMPPConnection(
-        connection_url,
+        getConnectionServiceURL(),
         Object.assign(
         Object.assign(
             _converse.default_connection_options,
             _converse.default_connection_options,
             api.settings.get("connection_options"),
             api.settings.get("connection_options"),

+ 0 - 1
src/headless/utils/core.js

@@ -27,7 +27,6 @@ export function isEmptyMessage (attrs) {
         !attrs['message'];
         !attrs['message'];
 }
 }
 
 
-
 /* We distinguish between UniView and MultiView instances.
 /* We distinguish between UniView and MultiView instances.
  *
  *
  * UniView means that only one chat is visible, even though there might be multiple ongoing chats.
  * UniView means that only one chat is visible, even though there might be multiple ongoing chats.

+ 24 - 8
src/headless/utils/init.js

@@ -291,25 +291,40 @@ export async function attemptNonPreboundSession (credentials, automatic) {
         // So we can't do the check (!automatic || _converse.api.settings.get("auto_login")) here.
         // So we can't do the check (!automatic || _converse.api.settings.get("auto_login")) here.
         if (credentials) {
         if (credentials) {
             connect(credentials);
             connect(credentials);
-        } else if (_converse.api.settings.get("credentials_url")) {
+        } else if (api.settings.get("credentials_url")) {
             // We give credentials_url preference, because
             // We give credentials_url preference, because
             // _converse.connection.pass might be an expired token.
             // _converse.connection.pass might be an expired token.
             connect(await getLoginCredentials());
             connect(await getLoginCredentials());
-        } else if (_converse.jid && (_converse.api.settings.get("password") || _converse.connection.pass)) {
+        } else if (_converse.jid && (api.settings.get("password") || _converse.connection.pass)) {
             connect();
             connect();
         } else if (!_converse.isTestEnv() && 'credentials' in navigator) {
         } else if (!_converse.isTestEnv() && 'credentials' in navigator) {
             connect(await getLoginCredentialsFromBrowser());
             connect(await getLoginCredentialsFromBrowser());
         } else {
         } else {
             !_converse.isTestEnv() && log.warn("attemptNonPreboundSession: Couldn't find credentials to log in with");
             !_converse.isTestEnv() && log.warn("attemptNonPreboundSession: Couldn't find credentials to log in with");
         }
         }
-    } else if ([_converse.ANONYMOUS, _converse.EXTERNAL].includes(_converse.api.settings.get("authentication")) && (!automatic || _converse.api.settings.get("auto_login"))) {
+    } else if (
+        [_converse.ANONYMOUS, _converse.EXTERNAL].includes(api.settings.get("authentication")) &&
+        (!automatic || api.settings.get("auto_login"))
+    ) {
         connect();
         connect();
     }
     }
 }
 }
 
 
 
 
+export function getConnectionServiceURL () {
+    const { api } = _converse;
+    if (('WebSocket' in window || 'MozWebSocket' in window) && api.settings.get("websocket_url")) {
+        return api.settings.get('websocket_url');
+    } else if (api.settings.get('bosh_service_url')) {
+        return api.settings.get('bosh_service_url');
+    }
+    return '';
+}
+
+
 function connect (credentials) {
 function connect (credentials) {
-    if ([_converse.ANONYMOUS, _converse.EXTERNAL].includes(_converse.api.settings.get("authentication"))) {
+    const { api } = _converse;
+    if ([_converse.ANONYMOUS, _converse.EXTERNAL].includes(api.settings.get("authentication"))) {
         if (!_converse.jid) {
         if (!_converse.jid) {
             throw new Error("Config Error: when using anonymous login " +
             throw new Error("Config Error: when using anonymous login " +
                 "you need to provide the server's domain via the 'jid' option. " +
                 "you need to provide the server's domain via the 'jid' option. " +
@@ -320,19 +335,20 @@ function connect (credentials) {
             _converse.connection.reset();
             _converse.connection.reset();
         }
         }
         _converse.connection.connect(_converse.jid.toLowerCase());
         _converse.connection.connect(_converse.jid.toLowerCase());
-    } else if (_converse.api.settings.get("authentication") === _converse.LOGIN) {
-        const password = credentials ? credentials.password : (_converse.connection?.pass || _converse.api.settings.get("password"));
+    } else if (api.settings.get("authentication") === _converse.LOGIN) {
+        const password = credentials?.password ?? (_converse.connection?.pass || api.settings.get("password"));
         if (!password) {
         if (!password) {
-            if (_converse.api.settings.get("auto_login")) {
+            if (api.settings.get("auto_login")) {
                 throw new Error("autoLogin: If you use auto_login and "+
                 throw new Error("autoLogin: If you use auto_login and "+
                     "authentication='login' then you also need to provide a password.");
                     "authentication='login' then you also need to provide a password.");
             }
             }
             _converse.connection.setDisconnectionCause(Strophe.Status.AUTHFAIL, undefined, true);
             _converse.connection.setDisconnectionCause(Strophe.Status.AUTHFAIL, undefined, true);
-            _converse.api.connection.disconnect();
+            api.connection.disconnect();
             return;
             return;
         }
         }
         if (!_converse.connection.reconnecting) {
         if (!_converse.connection.reconnecting) {
             _converse.connection.reset();
             _converse.connection.reset();
+            _converse.connection.service = getConnectionServiceURL();
         }
         }
         _converse.connection.connect(_converse.jid, password);
         _converse.connection.connect(_converse.jid, password);
     }
     }

+ 1 - 0
src/plugins/controlbox/index.js

@@ -58,6 +58,7 @@ converse.plugins.add('converse-controlbox', {
             allow_user_trust_override: true,
             allow_user_trust_override: true,
             default_domain: undefined,
             default_domain: undefined,
             locked_domain: undefined,
             locked_domain: undefined,
+            show_connection_url_input: false,
             show_controlbox_by_default: false,
             show_controlbox_by_default: false,
             sticky_controlbox: false
             sticky_controlbox: false
         });
         });

+ 33 - 69
src/plugins/controlbox/loginform.js

@@ -1,9 +1,8 @@
 import bootstrap from 'bootstrap.native';
 import bootstrap from 'bootstrap.native';
 import tpl_login_panel from './templates/loginform.js';
 import tpl_login_panel from './templates/loginform.js';
 import { CustomElement } from 'shared/components/element.js';
 import { CustomElement } from 'shared/components/element.js';
-import { Model } from '@converse/skeletor/src/model.js';
-import { __ } from 'i18n';
 import { _converse, api, converse } from '@converse/headless/core';
 import { _converse, api, converse } from '@converse/headless/core';
+import { updateSettingsWithFormData, validateJID } from './utils.js';
 
 
 const { Strophe, u } = converse.env;
 const { Strophe, u } = converse.env;
 
 
@@ -11,9 +10,18 @@ const { Strophe, u } = converse.env;
 class LoginForm extends CustomElement {
 class LoginForm extends CustomElement {
 
 
     initialize () {
     initialize () {
-        this.model = new Model();
         this.listenTo(_converse.connfeedback, 'change', () => this.requestUpdate());
         this.listenTo(_converse.connfeedback, 'change', () => this.requestUpdate());
-        this.listenTo(this.model, 'change', () => this.requestUpdate());
+        this.handler = () => this.requestUpdate()
+    }
+
+    connectedCallback () {
+        super.connectedCallback();
+        api.settings.listen.on('change', this.handler);
+    }
+
+    disconnectedCallback () {
+        super.disconnectedCallback();
+        api.settings.listen.not('change', this.handler);
     }
     }
 
 
     render () {
     render () {
@@ -26,21 +34,28 @@ class LoginForm extends CustomElement {
 
 
     async onLoginFormSubmitted (ev) {
     async onLoginFormSubmitted (ev) {
         ev?.preventDefault();
         ev?.preventDefault();
-        if (api.settings.get('bosh_service_url') ||
-                api.settings.get('websocket_url') ||
-                this.model.get('show_connection_url_input')) {
-            // The connection class will still try to discover XEP-0156 connection methods
-            this.authenticate(ev);
-        } else {
+
+        if (api.settings.get('authentication') === _converse.ANONYMOUS) {
+            return this.connect(_converse.jid);
+        }
+
+        if (!validateJID(ev.target)) {
+            return;
+        }
+        updateSettingsWithFormData(ev.target);
+
+        if (!api.settings.get('bosh_service_url') && !api.settings.get('websocket_url')) {
             // We don't have a connection URL available, so we try here to discover
             // We don't have a connection URL available, so we try here to discover
             // XEP-0156 connection methods now, and if not found we present the user
             // XEP-0156 connection methods now, and if not found we present the user
             // with the option to enter their own connection URL
             // with the option to enter their own connection URL
             await this.discoverConnectionMethods(ev);
             await this.discoverConnectionMethods(ev);
-            if (api.settings.get('bosh_service_url') || api.settings.get('websocket_url')) {
-                this.authenticate(ev);
-            } else {
-                this.model.set('show_connection_url_input', true);
-            }
+        }
+
+        if (api.settings.get('bosh_service_url') || api.settings.get('websocket_url')) {
+            // FIXME: The connection class will still try to discover XEP-0156 connection methods
+            this.connect();
+        } else {
+            api.settings.set('show_connection_url_input', true);
         }
         }
     }
     }
 
 
@@ -68,64 +83,13 @@ class LoginForm extends CustomElement {
         });
         });
     }
     }
 
 
-    validate () {
-        const form = this.querySelector('form');
-        const jid_element = form.querySelector('input[name=jid]');
-        if (
-            jid_element.value &&
-            !api.settings.get('locked_domain') &&
-            !api.settings.get('default_domain') &&
-            !u.isValidJID(jid_element.value)
-        ) {
-            jid_element.setCustomValidity(__('Please enter a valid XMPP address'));
-            return false;
-        }
-        jid_element.setCustomValidity('');
-        return true;
-    }
-
-    /**
-     * Authenticate the user based on a form submission event.
-     * @param { Event } ev
-     */
-    authenticate (ev) {
-        if (api.settings.get('authentication') === _converse.ANONYMOUS) {
-            return this.connect(_converse.jid, null);
-        }
-        if (!this.validate()) {
-            return;
-        }
-
-        const form_data = new FormData(ev.target);
-        const connection_url  = form_data.get('connection-url');
-        if (connection_url?.startsWith('ws')) {
-            api.settings.set('websocket_url', connection_url);
-        } else if (connection_url?.startsWith('http')) {
-            api.settings.set('bosh_service_url', connection_url);
-        }
-
-        _converse.config.save({ 'trusted': (form_data.get('trusted') && true) || false });
-
-        let jid = form_data.get('jid');
-        if (api.settings.get('locked_domain')) {
-            const last_part = '@' + api.settings.get('locked_domain');
-            if (jid.endsWith(last_part)) {
-                jid = jid.substr(0, jid.length - last_part.length);
-            }
-            jid = Strophe.escapeNode(jid) + last_part;
-        } else if (api.settings.get('default_domain') && !jid.includes('@')) {
-            jid = jid + '@' + api.settings.get('default_domain');
-        }
-        this.connect(jid, form_data.get('password'));
-    }
-
     // eslint-disable-next-line class-methods-use-this
     // eslint-disable-next-line class-methods-use-this
-    connect (jid, password) {
+    connect (jid) {
         if (['converse/login', 'converse/register'].includes(_converse.router.history.getFragment())) {
         if (['converse/login', 'converse/register'].includes(_converse.router.history.getFragment())) {
             _converse.router.navigate('', { 'replace': true });
             _converse.router.navigate('', { 'replace': true });
         }
         }
-        _converse.connection && _converse.connection.reset();
-        api.user.login(jid, password);
+        _converse.connection?.reset();
+        api.user.login(jid);
     }
     }
 }
 }
 
 

+ 3 - 1
src/plugins/controlbox/templates/loginform.js

@@ -84,6 +84,8 @@ const auth_fields = (el) => {
     const default_domain = api.settings.get('default_domain');
     const default_domain = api.settings.get('default_domain');
     const placeholder_username = ((locked_domain || default_domain) && __('Username')) || __('user@domain');
     const placeholder_username = ((locked_domain || default_domain) && __('Username')) || __('user@domain');
     const show_trust_checkbox = api.settings.get('allow_user_trust_override');
     const show_trust_checkbox = api.settings.get('allow_user_trust_override');
+    const show_connection_url_input = api.settings.get('show_connection_url_input')
+
     return html`
     return html`
         <div class="form-group">
         <div class="form-group">
             <label for="converse-login-jid">${i18n_xmpp_address}:</label>
             <label for="converse-login-jid">${i18n_xmpp_address}:</label>
@@ -98,7 +100,7 @@ const auth_fields = (el) => {
                 placeholder="${placeholder_username}"/>
                 placeholder="${placeholder_username}"/>
         </div>
         </div>
         ${ (authentication !== _converse.EXTERNAL) ? password_input() : '' }
         ${ (authentication !== _converse.EXTERNAL) ? password_input() : '' }
-        ${ el.model.get('show_connection_url_input') ? connection_url_input() : '' }
+        ${ show_connection_url_input ? connection_url_input() : '' }
         ${ show_trust_checkbox ? trust_checkbox(show_trust_checkbox === 'off' ? false : true) : '' }
         ${ show_trust_checkbox ? trust_checkbox(show_trust_checkbox === 'off' ? false : true) : '' }
         <fieldset class="form-group buttons">
         <fieldset class="form-group buttons">
             <input class="btn btn-primary" type="submit" value="${i18n_login}"/>
             <input class="btn btn-primary" type="submit" value="${i18n_login}"/>

+ 56 - 2
src/plugins/controlbox/utils.js

@@ -1,6 +1,7 @@
-import { _converse, converse } from "@converse/headless/core";
+import { __ } from 'i18n';
+import { _converse, api, converse } from "@converse/headless/core";
 
 
-const u = converse.env.utils;
+const { Strophe, u } = converse.env;
 
 
 export function addControlBox () {
 export function addControlBox () {
     const m = _converse.chatboxes.add(new _converse.ControlBox({'id': 'controlbox'}));
     const m = _converse.chatboxes.add(new _converse.ControlBox({'id': 'controlbox'}));
@@ -46,3 +47,56 @@ export function onChatBoxesFetched () {
     const controlbox = _converse.chatboxes.get('controlbox') || addControlBox();
     const controlbox = _converse.chatboxes.get('controlbox') || addControlBox();
     controlbox.save({ 'connected': true });
     controlbox.save({ 'connected': true });
 }
 }
+
+
+/**
+ * Given the login `<form>` element, parse its data and update the
+ * converse settings with the supplied JID, password and connection URL.
+ * @param { HTMLElement } form
+ * @param { Object } settings - Extra settings that may be passed in and will
+ *  also be set together with the form settings.
+ */
+export function updateSettingsWithFormData (form, settings={}) {
+    const form_data = new FormData(form);
+
+    const connection_url  = form_data.get('connection-url');
+    if (connection_url?.startsWith('ws')) {
+        settings['websocket_url'] = connection_url;
+    } else if (connection_url?.startsWith('http')) {
+        settings['bosh_service_url'] = connection_url;
+    }
+
+    let jid = form_data.get('jid');
+    if (api.settings.get('locked_domain')) {
+        const last_part = '@' + api.settings.get('locked_domain');
+        if (jid.endsWith(last_part)) {
+            jid = jid.substr(0, jid.length - last_part.length);
+        }
+        jid = Strophe.escapeNode(jid) + last_part;
+    } else if (api.settings.get('default_domain') && !jid.includes('@')) {
+        jid = jid + '@' + api.settings.get('default_domain');
+    }
+    settings['jid'] = jid;
+    settings['password'] = form_data.get('password');
+
+    api.settings.set(settings);
+
+    _converse.config.save({ 'trusted': (form_data.get('trusted') && true) || false });
+}
+
+
+export function validateJID (form) {
+    const jid_element = form.querySelector('input[name=jid]');
+    if (
+        jid_element.value &&
+        !api.settings.get('locked_domain') &&
+        !api.settings.get('default_domain') &&
+        !u.isValidJID(jid_element.value)
+    ) {
+        jid_element.setCustomValidity(__('Please enter a valid XMPP address'));
+        return false;
+    }
+    jid_element.setCustomValidity('');
+    return true;
+}
+