[pve-devel] [PATCH manager 1/1] dc/TFAEdit: rewrite enabling/disabling logic using userid/tfa api call
Thomas Lamprecht
t.lamprecht at proxmox.com
Fri Apr 19 07:02:58 CEST 2019
Am 4/18/19 um 12:46 PM schrieb Dominik Csapak:
> with the new /access/users/{userid}/tfa api call, we do not need
> to pass the tfa_type from the userview anymore
>
> but the whole binding/formula handling was not really nice and
not sure if the whole big me.down/lookup block you replaced it with is nicer,
the bindings where actually in the element scopes definitions, so one could see
the interworkings faster.. Anyway, I'd say the "nice-ness" is here prob. striclty
subjective, but the viewModel mtehod was objective worse, IMO.
I'll take a closer look at this next week, though :)
> unnecessary since we set a viewmodel, which triggered a formula, which
> triggered an enabling/disabling of one component
>
> instead of that, simply enable or disable the component directly
> (especially since this was only called once anyway)
>
> keep the viewmodel for things that can change dynamically, e.g. the
> tab or the validity
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> www/manager6/dc/TFAEdit.js | 163 +++++++++++++++++++++-----------------------
> www/manager6/dc/UserView.js | 1 -
> 2 files changed, 77 insertions(+), 87 deletions(-)
>
> diff --git a/www/manager6/dc/TFAEdit.js b/www/manager6/dc/TFAEdit.js
> index c57c5339..91b14007 100644
> --- a/www/manager6/dc/TFAEdit.js
> +++ b/www/manager6/dc/TFAEdit.js
> @@ -99,48 +99,66 @@ Ext.define('PVE.window.TFAEdit', {
> viewModel: {
> data: {
> in_totp_tab: true,
> - tfa_required: false,
> - tfa_type: undefined,
> - valid: false,
> - u2f_available: true
> - },
> - formulas: {
> - canDeleteTFA: function(get) {
> - return (get('tfa_type') !== undefined && !get('tfa_required'));
> - },
> - canSetupTOTP: function(get) {
> - var tfa = get('tfa_type');
> - return (tfa === undefined || tfa === 'totp' || tfa === 1);
> - },
> - canSetupU2F: function(get) {
> - var tfa = get('tfa_type');
> - return (get('u2f_available') && (tfa === undefined || tfa === 'u2f' || tfa === 1));
> - }
> + valid: false
> }
> },
>
> - afterLoadingRealm: function(realm_tfa_type) {
> + afterLoading: function(realm_type, user_type) {
> var me = this;
> - var viewmodel = me.getViewModel();
> - if (!realm_tfa_type) {
> - // There's no TFA enforced by the realm, everything works.
> - viewmodel.set('u2f_available', true);
> - viewmodel.set('tfa_required', false);
> - } else if (realm_tfa_type === 'oath') {
> - // The realm explicitly requires TOTP
> - viewmodel.set('tfa_required', true);
> - viewmodel.set('u2f_available', false);
> - } else {
> + var controller = me.getController();
> +
> + // exit early
> + if (realm_type && realm_type !== 'oath') {
> // The realm enforces some other TFA type (yubico)
> me.close();
> Ext.Msg.alert(
> gettext('Error'),
> Ext.String.format(
> gettext("Custom 2nd factor configuration is not supported on realms with '{0}' TFA."),
> - realm_tfa_type
> + realm_type
> )
> );
> + return;
> + }
> +
> + var can_setup_totp = (realm_type === 'oath' || !user_type || user_type === 'oath');
> + var can_setup_u2f = (!realm_type && (!user_type || user_type === 'u2f'));
> + var can_delete = (!realm_type && !!user_type);
> +
> + // disable tabs/buttons/labels
> + me.lookup('totp_panel').setDisabled(!can_setup_totp);
> + me.lookup('u2f_panel').setDisabled(!can_setup_u2f);
> + me.lookup('delete_button').setDisabled(!can_delete);
> + me.lookup('register_button').setDisabled(user_type === 'u2f');
> + me.lookup('delete_label').setVisible(user_type === 'u2f');
> + me.lookup('register_label').setVisible(user_type !== 'u2f');
> +
> + me.qrdiv = document.createElement('center');
> + me.qrcode = new QRCode(me.qrdiv, {
> + width: 256,
> + height: 256,
> + correctLevel: QRCode.CorrectLevel.M
> + });
> + me.down('#qrbox').getEl().appendChild(me.qrdiv);
> +
> + if (!user_type || (user_type !== 'oath' && realm_type === 'oath')) {
> + controller.randomizeSecret();
> + } else {
> + me.down('#qrbox').setVisible(false);
> + me.lookup('challenge').setVisible(false);
> + }
> +
> + // change tab if totp is not available but u2f is
> + if (!can_setup_totp && can_setup_u2f) {
> + var u2f_panel = me.lookup('u2f_panel');
> + me.lookup('tfatabs').setActiveTab(u2f_panel);
> + }
> +
> + if (Proxmox.UserName === 'root at pam') {
> + me.lookup('password').setVisible(false);
> + me.lookup('password').setDisabled(true);
> }
> +
> },
>
> controller: {
> @@ -162,41 +180,10 @@ Ext.define('PVE.window.TFAEdit', {
> viewModel.set('valid', form.isValid() && challenge.isValid() && password.isValid());
> }
> },
> - '#': {
> - show: function() {
> - var me = this.getView();
> - var viewmodel = this.getViewModel();
> -
> - me.qrdiv = document.createElement('center');
> - me.qrcode = new QRCode(me.qrdiv, {
> - width: 256,
> - height: 256,
> - correctLevel: QRCode.CorrectLevel.M
> - });
> - me.down('#qrbox').getEl().appendChild(me.qrdiv);
> -
> - viewmodel.set('tfa_type', me.tfa_type);
> - if (!me.tfa_type) {
> - this.randomizeSecret();
> - } else {
> - me.down('#qrbox').setVisible(false);
> - me.lookup('challenge').setVisible(false);
> - if (me.tfa_type === 'u2f') {
> - var u2f_panel = me.lookup('u2f_panel');
> - me.lookup('tfatabs').setActiveTab(u2f_panel);
> - }
> - }
> -
> - if (Proxmox.UserName === 'root at pam') {
> - me.lookup('password').setVisible(false);
> - me.lookup('password').setDisabled(true);
> - }
> - }
> - },
> '#tfatabs': {
> tabchange: function(panel, newcard) {
> var viewmodel = this.getViewModel();
> - viewmodel.set('in_totp_tab', newcard.itemId === 'totp-panel');
> + viewmodel.set('in_totp_tab', newcard.reference === 'totp_panel');
> }
> }
> },
> @@ -317,12 +304,9 @@ Ext.define('PVE.window.TFAEdit', {
> {
> xtype: 'panel',
> title: 'TOTP',
> - itemId: 'totp-panel',
> + reference: 'totp_panel',
> tfa_type: 'totp',
> border: false,
> - bind: {
> - disabled: '{!canSetupTOTP}'
> - },
> layout: {
> type: 'vbox',
> align: 'stretch'
> @@ -422,7 +406,6 @@ Ext.define('PVE.window.TFAEdit', {
> },
> {
> title: 'U2F',
> - itemId: 'u2f-panel',
> reference: 'u2f_panel',
> tfa_type: 'u2f',
> border: false,
> @@ -431,14 +414,19 @@ Ext.define('PVE.window.TFAEdit', {
> type: 'vbox',
> align: 'middle'
> },
> - bind: {
> - disabled: '{!canSetupU2F}'
> - },
> items: [
> {
> xtype: 'label',
> + reference: 'register_label',
> width: 500,
> text: gettext('To register a U2F device, connect the device, then click the button and follow the instructions.')
> + },
> + {
> + xtype: 'label',
> + hidden: true,
> + reference: 'delete_label',
> + width: 500,
> + text: gettext('To register a new U2F device, first delete the Information of the old one.')
> }
> ]
> }
> @@ -472,41 +460,44 @@ Ext.define('PVE.window.TFAEdit', {
> },
> {
> xtype: 'button',
> + reference: 'register_button',
> text: gettext('Register U2F Device'),
> handler: 'startU2FRegistration',
> bind: {
> - hidden: '{in_totp_tab}',
> - disabled: '{tfa_type}'
> + hidden: '{in_totp_tab}'
> }
> },
> {
> text: gettext('Delete'),
> reference: 'delete_button',
> disabled: true,
> - handler: 'deleteTFA',
> - bind: {
> - disabled: '{!canDeleteTFA}'
> - }
> + handler: 'deleteTFA'
> }
> ],
>
> initComponent: function() {
> var me = this;
>
> - var store = new Ext.data.Store({
> - model: 'pve-domains',
> - autoLoad: true
> - });
> + if (!me.userid) {
> + throw "no userid given";
> + }
>
> - store.on('load', function() {
> - var user_realm = me.userid.split('@')[1];
> - var realm = me.store.findRecord('realm', user_realm);
> - me.afterLoadingRealm(realm && realm.data && realm.data.tfa);
> - }, me);
> + me.callParent();
>
> - Ext.apply(me, { store: store });
> + Proxmox.Utils.setErrorMask(me.down('#tfatabs'), true);
>
> - me.callParent();
> + Proxmox.Utils.API2Request({
> + url: '/access/users/' + encodeURIComponent(me.userid) + '/tfa',
> + method: 'GET',
> + success: function(response, opts) {
> + var data = response.result.data;
> + Proxmox.Utils.setErrorMask(me.down('#tfatabs'), false);
> + me.afterLoading(data.realm, data.user);
> + },
> + failure: function(response, opts) {
> + Proxmox.Utils.setErrorMask(me, response.htmlStatus);
> + }
> + });
>
> Ext.GlobalEvents.fireEvent('proxmoxShowHelp', 'pveum_tfa_auth');
> }
> diff --git a/www/manager6/dc/UserView.js b/www/manager6/dc/UserView.js
> index 8918fb2b..3c6d1a93 100644
> --- a/www/manager6/dc/UserView.js
> +++ b/www/manager6/dc/UserView.js
> @@ -87,7 +87,6 @@ Ext.define('PVE.dc.UserView', {
> var d = rec.data;
> var tfa_type = PVE.Parser.parseTfaType(d.keys);
> var win = Ext.create('PVE.window.TFAEdit',{
> - tfa_type: tfa_type,
> userid: d.userid
> });
> win.on('destroy', reload);
>
More information about the pve-devel
mailing list