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.csa...@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@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@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);
> 


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to