nextcloud /
server
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like KeyManager often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use KeyManager, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 39 | class KeyManager { |
||
| 40 | |||
| 41 | /** |
||
| 42 | * @var Session |
||
| 43 | */ |
||
| 44 | protected $session; |
||
| 45 | /** |
||
| 46 | * @var IStorage |
||
| 47 | */ |
||
| 48 | private $keyStorage; |
||
| 49 | /** |
||
| 50 | * @var Crypt |
||
| 51 | */ |
||
| 52 | private $crypt; |
||
| 53 | /** |
||
| 54 | * @var string |
||
| 55 | */ |
||
| 56 | private $recoveryKeyId; |
||
| 57 | /** |
||
| 58 | * @var string |
||
| 59 | */ |
||
| 60 | private $publicShareKeyId; |
||
| 61 | /** |
||
| 62 | * @var string |
||
| 63 | */ |
||
| 64 | private $masterKeyId; |
||
| 65 | /** |
||
| 66 | * @var string UserID |
||
| 67 | */ |
||
| 68 | private $keyId; |
||
| 69 | /** |
||
| 70 | * @var string |
||
| 71 | */ |
||
| 72 | private $publicKeyId = 'publicKey'; |
||
| 73 | /** |
||
| 74 | * @var string |
||
| 75 | */ |
||
| 76 | private $privateKeyId = 'privateKey'; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * @var string |
||
| 80 | */ |
||
| 81 | private $shareKeyId = 'shareKey'; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * @var string |
||
| 85 | */ |
||
| 86 | private $fileKeyId = 'fileKey'; |
||
| 87 | /** |
||
| 88 | * @var IConfig |
||
| 89 | */ |
||
| 90 | private $config; |
||
| 91 | /** |
||
| 92 | * @var ILogger |
||
| 93 | */ |
||
| 94 | private $log; |
||
| 95 | /** |
||
| 96 | * @var Util |
||
| 97 | */ |
||
| 98 | private $util; |
||
| 99 | |||
| 100 | /** |
||
| 101 | * @param IStorage $keyStorage |
||
| 102 | * @param Crypt $crypt |
||
| 103 | * @param IConfig $config |
||
| 104 | * @param IUserSession $userSession |
||
| 105 | * @param Session $session |
||
| 106 | * @param ILogger $log |
||
| 107 | * @param Util $util |
||
| 108 | */ |
||
| 109 | public function __construct( |
||
| 152 | |||
| 153 | /** |
||
| 154 | * check if key pair for public link shares exists, if not we create one |
||
| 155 | */ |
||
| 156 | public function validateShareKey() { |
||
| 172 | |||
| 173 | /** |
||
| 174 | * check if a key pair for the master key exists, if not we create one |
||
| 175 | */ |
||
| 176 | public function validateMasterKey() { |
||
| 206 | |||
| 207 | /** |
||
| 208 | * @return bool |
||
| 209 | */ |
||
| 210 | public function recoveryKeyExists() { |
||
| 214 | |||
| 215 | /** |
||
| 216 | * get recovery key |
||
| 217 | * |
||
| 218 | * @return string |
||
| 219 | */ |
||
| 220 | public function getRecoveryKey() { |
||
| 223 | |||
| 224 | /** |
||
| 225 | * get recovery key ID |
||
| 226 | * |
||
| 227 | * @return string |
||
| 228 | */ |
||
| 229 | public function getRecoveryKeyId() { |
||
| 232 | |||
| 233 | /** |
||
| 234 | * @param string $password |
||
| 235 | * @return bool |
||
| 236 | */ |
||
| 237 | public function checkRecoveryPassword($password) { |
||
| 246 | |||
| 247 | /** |
||
| 248 | * @param string $uid |
||
| 249 | * @param string $password |
||
| 250 | * @param string $keyPair |
||
| 251 | * @return bool |
||
| 252 | */ |
||
| 253 | public function storeKeyPair($uid, $password, $keyPair) { |
||
| 267 | |||
| 268 | /** |
||
| 269 | * @param string $password |
||
| 270 | * @param array $keyPair |
||
| 271 | * @return bool |
||
| 272 | */ |
||
| 273 | public function setRecoveryKey($password, $keyPair) { |
||
| 289 | |||
| 290 | /** |
||
| 291 | * @param $userId |
||
| 292 | * @param $key |
||
| 293 | * @return bool |
||
| 294 | */ |
||
| 295 | public function setPublicKey($userId, $key) { |
||
| 298 | |||
| 299 | /** |
||
| 300 | * @param $userId |
||
| 301 | * @param string $key |
||
| 302 | * @return bool |
||
| 303 | */ |
||
| 304 | public function setPrivateKey($userId, $key) { |
||
| 310 | |||
| 311 | /** |
||
| 312 | * write file key to key storage |
||
| 313 | * |
||
| 314 | * @param string $path |
||
| 315 | * @param string $key |
||
| 316 | * @return boolean |
||
| 317 | */ |
||
| 318 | public function setFileKey($path, $key) { |
||
| 321 | |||
| 322 | /** |
||
| 323 | * set all file keys (the file key and the corresponding share keys) |
||
| 324 | * |
||
| 325 | * @param string $path |
||
| 326 | * @param array $keys |
||
| 327 | */ |
||
| 328 | public function setAllFileKeys($path, $keys) { |
||
| 334 | |||
| 335 | /** |
||
| 336 | * write share key to the key storage |
||
| 337 | * |
||
| 338 | * @param string $path |
||
| 339 | * @param string $uid |
||
| 340 | * @param string $key |
||
| 341 | * @return boolean |
||
| 342 | */ |
||
| 343 | public function setShareKey($path, $uid, $key) { |
||
| 347 | |||
| 348 | /** |
||
| 349 | * Decrypt private key and store it |
||
| 350 | * |
||
| 351 | * @param string $uid user id |
||
| 352 | * @param string $passPhrase users password |
||
| 353 | * @return boolean |
||
| 354 | */ |
||
| 355 | public function init($uid, $passPhrase) { |
||
| 389 | |||
| 390 | /** |
||
| 391 | * @param $userId |
||
| 392 | * @return string |
||
| 393 | * @throws PrivateKeyMissingException |
||
| 394 | */ |
||
| 395 | View Code Duplication | public function getPrivateKey($userId) { |
|
| 404 | |||
| 405 | /** |
||
| 406 | * @param string $path |
||
| 407 | * @param $uid |
||
| 408 | * @return string |
||
| 409 | */ |
||
| 410 | public function getFileKey($path, $uid) { |
||
| 450 | |||
| 451 | /** |
||
| 452 | * Get the current version of a file |
||
| 453 | * |
||
| 454 | * @param string $path |
||
| 455 | * @param View $view |
||
| 456 | * @return int |
||
| 457 | */ |
||
| 458 | public function getVersion($path, View $view) { |
||
| 465 | |||
| 466 | /** |
||
| 467 | * Set the current version of a file |
||
| 468 | * |
||
| 469 | * @param string $path |
||
| 470 | * @param int $version |
||
| 471 | * @param View $view |
||
| 472 | */ |
||
| 473 | public function setVersion($path, $version, View $view) { |
||
| 481 | |||
| 482 | /** |
||
| 483 | * get the encrypted file key |
||
| 484 | * |
||
| 485 | * @param string $path |
||
| 486 | * @return string |
||
| 487 | */ |
||
| 488 | public function getEncryptedFileKey($path) { |
||
| 494 | |||
| 495 | /** |
||
| 496 | * delete share key |
||
| 497 | * |
||
| 498 | * @param string $path |
||
| 499 | * @param string $keyId |
||
| 500 | * @return boolean |
||
| 501 | */ |
||
| 502 | public function deleteShareKey($path, $keyId) { |
||
| 508 | |||
| 509 | |||
| 510 | /** |
||
| 511 | * @param $path |
||
| 512 | * @param $uid |
||
| 513 | * @return mixed |
||
| 514 | */ |
||
| 515 | public function getShareKey($path, $uid) { |
||
| 519 | |||
| 520 | /** |
||
| 521 | * check if user has a private and a public key |
||
| 522 | * |
||
| 523 | * @param string $userId |
||
| 524 | * @return bool |
||
| 525 | * @throws PrivateKeyMissingException |
||
| 526 | * @throws PublicKeyMissingException |
||
| 527 | */ |
||
| 528 | public function userHasKeys($userId) { |
||
| 553 | |||
| 554 | /** |
||
| 555 | * @param $userId |
||
| 556 | * @return mixed |
||
| 557 | * @throws PublicKeyMissingException |
||
| 558 | */ |
||
| 559 | View Code Duplication | public function getPublicKey($userId) { |
|
| 567 | |||
| 568 | public function getPublicShareKeyId() { |
||
| 571 | |||
| 572 | /** |
||
| 573 | * get public key for public link shares |
||
| 574 | * |
||
| 575 | * @return string |
||
| 576 | */ |
||
| 577 | public function getPublicShareKey() { |
||
| 580 | |||
| 581 | /** |
||
| 582 | * @param string $purpose |
||
| 583 | * @param string $uid |
||
| 584 | */ |
||
| 585 | public function backupUserKeys($purpose, $uid) { |
||
| 588 | |||
| 589 | /** |
||
| 590 | * creat a backup of the users private and public key and then delete it |
||
| 591 | * |
||
| 592 | * @param string $uid |
||
| 593 | */ |
||
| 594 | public function deleteUserKeys($uid) { |
||
| 598 | |||
| 599 | /** |
||
| 600 | * @param $uid |
||
| 601 | * @return bool |
||
| 602 | */ |
||
| 603 | public function deletePublicKey($uid) { |
||
| 606 | |||
| 607 | /** |
||
| 608 | * @param string $uid |
||
| 609 | * @return bool |
||
| 610 | */ |
||
| 611 | private function deletePrivateKey($uid) { |
||
| 614 | |||
| 615 | /** |
||
| 616 | * @param string $path |
||
| 617 | * @return bool |
||
| 618 | */ |
||
| 619 | public function deleteAllFileKeys($path) { |
||
| 622 | |||
| 623 | /** |
||
| 624 | * @param array $userIds |
||
| 625 | * @return array |
||
| 626 | * @throws PublicKeyMissingException |
||
| 627 | */ |
||
| 628 | public function getPublicKeys(array $userIds) { |
||
| 642 | |||
| 643 | /** |
||
| 644 | * @param string $keyId |
||
| 645 | * @return string returns openssl key |
||
| 646 | */ |
||
| 647 | public function getSystemPrivateKey($keyId) { |
||
| 650 | |||
| 651 | /** |
||
| 652 | * @param string $keyId |
||
| 653 | * @param string $key |
||
| 654 | * @return string returns openssl key |
||
| 655 | */ |
||
| 656 | public function setSystemPrivateKey($keyId, $key) { |
||
| 662 | |||
| 663 | /** |
||
| 664 | * add system keys such as the public share key and the recovery key |
||
| 665 | * |
||
| 666 | * @param array $accessList |
||
| 667 | * @param array $publicKeys |
||
| 668 | * @param string $uid |
||
| 669 | * @return array |
||
| 670 | * @throws PublicKeyMissingException |
||
| 671 | */ |
||
| 672 | public function addSystemKeys(array $accessList, array $publicKeys, $uid) { |
||
| 689 | |||
| 690 | /** |
||
| 691 | * get master key password |
||
| 692 | * |
||
| 693 | * @return string |
||
| 694 | * @throws \Exception |
||
| 695 | */ |
||
| 696 | public function getMasterKeyPassword() { |
||
| 704 | |||
| 705 | /** |
||
| 706 | * return master key id |
||
| 707 | * |
||
| 708 | * @return string |
||
| 709 | */ |
||
| 710 | public function getMasterKeyId() { |
||
| 713 | |||
| 714 | /** |
||
| 715 | * get public master key |
||
| 716 | * |
||
| 717 | * @return string |
||
| 718 | */ |
||
| 719 | public function getPublicMasterKey() { |
||
| 722 | } |
||
| 723 |
Loading history...
This check looks for type mismatches where the missing type is
false. This is usually indicative of an error condtion.Consider the follow example
This function either returns a new
DateTimeobject or false, if there was an error. This is a typical pattern in PHP programming to show that an error has occurred without raising an exception. The calling code should check for this returnedfalsebefore passing on the value to another function or method that may not be able to handle afalse.