Fix warning when resetting user's password with masterkey encryption

[pako81]

Description

Suppress warning when resetting user's password over the Users management page with masterkey encryption enabled.

Related Issue

• Fixes https://github.com/owncloud/enterprise/issues/3647

Motivation and Context

This PR fixes the following warning displayed when the admin is about to reset user's password over the Users management page with masterkey encryption in place:

The warning itself does not prevent resetting user's password but it is scary and makes no sense when masterkey encryption is in use.

How Has This Been Tested?

Manually by enabling masterkey encryption and as admin hovering over the password field for one random user: the warning is not displayed anymore.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised
• Changelog item

[phil-davis]

Unit test expectations also need adjusting: https://drone.owncloud.com/owncloud/core/21714/13/9

[codecov]

Codecov Report

Merging #36523 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #36523      +/-   ##
============================================
- Coverage     64.67%   64.67%   -0.01%     
+ Complexity    19099    19098       -1     
============================================
  Files          1268     1268              
  Lines         74695    74693       -2     
  Branches       1320     1320              
============================================
- Hits          48309    48307       -2     
  Misses        25998    25998              
  Partials        388      388

Flag	Coverage Δ	Complexity Δ
#javascript	54.12% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	65.84% <100%> (-0.01%)	19098 <0> (-1)

Impacted Files	Coverage Δ	Complexity Δ
settings/Controller/UsersController.php	85.51% <100%> (-0.06%)	131 <0> (-1)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e08811...6e31d51. Read the comment docs.

[sharidas]

Lots of if's and else would bring down the readability. How about changing this code to:

if ($this->isRestoreEnabled) {
	$recoveryMode = $this->config->getUserValue($user->getUID(), 'encryption', 'recoveryEnabled', '0');
	// method call inside empty is possible with PHP 5.5+
	$recoveryModeEnabled = !empty($recoveryMode);
	if ($recoveryModeEnabled) {
		// user also has recovery mode enabled
		$restorePossible = true;
	}
} else {
	$restorePossible = true;
}

[pako81]

@sharidas thanks for the suggestion!

I guess we can indeed get rid of the if (this->isEncryptionAppEnabled) condition since if ($this->isRestoreEnabled) is basically already performing the same check. This will help us to have better code readability.

Got also rid of any reference to isEncryptionAppEnabled from code.

[sharidas]

There was a semi colon added in the proposesd change, due to which the tests were failing. I have removed the semi colon and now the tests should pass.

[sharidas]

Now the code looks good to me. I have not tested this. If it works as expected, then Ok from my side.

[phil-davis]

I rebased just now to make sure that CI is still good for this.

@micbar is this to be released?
Needs someone to review/approve.
I added to 10.4 project so it gets noticed - otherwise these PRs seem to just sit and nobody looks at them.

[sharidas]

Tests done

• With masterkey encryption enabled

  • Create a user in the users page and try to reset the password for the new user. The message related to data recovery is not shown.

• With user keys encryption

  • Create a user in the users page and try to enable the recovery password for the user.
    • Now try to reset the password. There is no message displayed related to data recovery.
  • Create another user in the users page and do not enable the recovery password for the user
    • Now try to reset the password. There is a message that appears. This is expected behaviour.

• Tested without encryption

  • All users created in the users page can reset their password gracefully and the data recovery message does not pops up. This behaves as expected.

[sharidas]

LGTM 👍

[pako81]

thanks @phil-davis @sharidas @micbar