born05 / craft-twofactorauthentication

Craft plugin for two-factor or two-step login using Time Based OTP.
MIT License
36 stars 26 forks source link

None admin user given permission to `editUsers` seeing 500 error when add two-factor authentication as a column #55

Closed xhuang9 closed 1 year ago

xhuang9 commented 3 years ago

None admin user given permission to manage other craft user seeing 500 error when adding two-factor Authentication as a column in EVENT_SET_TABLE_ATTRIBUTE_HTML

Error: [error][Twig\Error\RuntimeError] yii\base\UnknownPropertyException: Getting unknown property: craft\elements\User::hasTwoFactorAuthentication in /vendor/yiisoft/yii2/base/Component.php:155

Fix description: If a user given access to editUsers and have permission to the accessPlugin-two-factor-authentication, they should see what admin is seeing.

Fixes 1: Allow none admin who can editUsers see the two-factor authentication column without an 500 error https://github.com/born05/craft-twofactorauthentication/blob/060bdad53fb92527a6a85fcfc68fd0c679b48f82/src/Plugin.php#L102 update to

if( $event->attribute == 'hasTwoFactorAuthentication' 
 && $currentUser->can('editUsers')
 && $currentUser->can('accessPlugin-two-factor-authentication') )

Optional Fixes 2: Allow none admin who can editUsers to disable the two-factor for other user https://github.com/born05/craft-twofactorauthentication/blob/060bdad53fb92527a6a85fcfc68fd0c679b48f82/src/Plugin.php#L121 update to

$currentUser = Craft::$app->getUser()->getIdentity();
if( $currentUser->can('editUsers')
 && $currentUser->can('accessPlugin-two-factor-authentication')
 && !$context['isNewUser'] ){

https://github.com/born05/craft-twofactorauthentication/blob/060bdad53fb92527a6a85fcfc68fd0c679b48f82/src/controllers/UsersController.php#L21 update to

$currentUser = Craft::$app->user->getIdentity();
if ($currentUser->can('editUsers') && $currentUser->can('accessPlugin-two-factor-authentication')) {
mike-moreau commented 1 year ago

@roelvanhintum, this 500 error has been an issue for us also. It would be nice to have it resolved for a project we're in process on. Would it help if we submitted a pull request?

roelvanhintum commented 1 year ago

@mike-moreau yes, that would definitely help.

mike-moreau commented 1 year ago

@roelvanhintum sure thing, made a PR here https://github.com/born05/craft-twofactorauthentication/pull/73 with the fixes suggested by @xhuang9 above

roelvanhintum commented 1 year ago

Sorry about the delay. Released in 3.1.0