Refactors lib/private/Security.

Mainly using PHP8's constructor property promotion.

Signed-off-by: Faraz Samapoor <fsa@adlas.at>

lib/private/Security/Certificate.php

@@ -30,25 +30,24 @@ namespace OC\Security;
 use OCP\ICertificate;
 
 class Certificate implements ICertificate {
-	protected $name;
+	protected string $name;
 
-	protected $commonName;
+	protected mixed $commonName;
 
-	protected $organization;
+	protected mixed $organization;
 
 	protected $serial;
 
-	protected $issueDate;
+	protected \DateTime $issueDate;
 
-	protected $expireDate;
+	protected \DateTime $expireDate;
 
-	protected $issuerName;
+	protected mixed $issuerName;
 
-	protected $issuerOrganization;
+	protected mixed $issuerOrganization;
 
 	/**
 	 * @param string $data base64 encoded certificate
-	 * @param string $name
 	 * @throws \Exception If the certificate could not get parsed
 	 */
 	public function __construct(string $data, string $name) {
@@ -66,67 +65,43 @@ class Certificate implements ICertificate {
 			throw new \Exception('Certificate could not get parsed.');
 		}
 
-		$this->commonName = isset($info['subject']['CN']) ? $info['subject']['CN'] : null;
-		$this->organization = isset($info['subject']['O']) ? $info['subject']['O'] : null;
+		$this->commonName = $info['subject']['CN'] ?? null;
+		$this->organization = $info['subject']['O'] ?? null;
 		$this->issueDate = new \DateTime('@' . $info['validFrom_time_t'], $gmt);
 		$this->expireDate = new \DateTime('@' . $info['validTo_time_t'], $gmt);
-		$this->issuerName = isset($info['issuer']['CN']) ? $info['issuer']['CN'] : null;
-		$this->issuerOrganization = isset($info['issuer']['O']) ? $info['issuer']['O'] : null;
+		$this->issuerName = $info['issuer']['CN'] ?? null;
+		$this->issuerOrganization = $info['issuer']['O'] ?? null;
 	}
 
-	/**
-	 * @return string
-	 */
 	public function getName(): string {
 		return $this->name;
 	}
 
-	/**
-	 * @return string|null
-	 */
 	public function getCommonName(): ?string {
 		return $this->commonName;
 	}
 
-	/**
-	 * @return string|null
-	 */
 	public function getOrganization(): ?string {
 		return $this->organization;
 	}
 
-	/**
-	 * @return \DateTime
-	 */
 	public function getIssueDate(): \DateTime {
 		return $this->issueDate;
 	}
 
-	/**
-	 * @return \DateTime
-	 */
 	public function getExpireDate(): \DateTime {
 		return $this->expireDate;
 	}
 
-	/**
-	 * @return bool
-	 */
 	public function isExpired(): bool {
 		$now = new \DateTime();
 		return $this->issueDate > $now or $now > $this->expireDate;
 	}
 
-	/**
-	 * @return string|null
-	 */
 	public function getIssuerName(): ?string {
 		return $this->issuerName;
 	}
 
-	/**
-	 * @return string|null
-	 */
 	public function getIssuerOrganization(): ?string {
 		return $this->issuerOrganization;
 	}

lib/private/Security/CertificateManager.php

@@ -44,21 +44,14 @@ use Psr\Log\LoggerInterface;
  * Manage trusted certificates for users
  */
 class CertificateManager implements ICertificateManager {
-	protected View $view;
-	protected IConfig $config;
-	protected LoggerInterface $logger;
-	protected ISecureRandom $random;
-
 	private ?string $bundlePath = null;
 
-	public function __construct(View $view,
-								IConfig $config,
-								LoggerInterface $logger,
-								ISecureRandom $random) {
-		$this->view = $view;
-		$this->config = $config;
-		$this->logger = $logger;
-		$this->random = $random;
+	public function __construct(
+		protected View $view,
+		protected IConfig $config,
+		protected LoggerInterface $logger,
+		protected ISecureRandom $random,
+	) {
 	}
 
 	/**
@@ -178,7 +171,6 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @param string $certificate the certificate data
 	 * @param string $name the filename for the certificate
-	 * @return \OCP\ICertificate
 	 * @throws \Exception If the certificate could not get added
 	 */
 	public function addCertificate(string $certificate, string $name): ICertificate {
@@ -205,9 +197,6 @@ class CertificateManager implements ICertificateManager {
 
 	/**
 	 * Remove the certificate and re-generate the certificate bundle
-	 *
-	 * @param string $name
-	 * @return bool
 	 */
 	public function removeCertificate(string $name): bool {
 		if (!Filesystem::isValidPath($name)) {
@@ -225,8 +214,6 @@ class CertificateManager implements ICertificateManager {
 
 	/**
 	 * Get the path to the certificate bundle
-	 *
-	 * @return string
 	 */
 	public function getCertificateBundle(): string {
 		return $this->getPathToCertificates() . 'rootcerts.crt';
@@ -267,8 +254,6 @@ class CertificateManager implements ICertificateManager {
 
 	/**
 	 * Check if we need to re-bundle the certificates because one of the sources has updated
-	 *
-	 * @return bool
 	 */
 	private function needsRebundling(): bool {
 		$targetBundle = $this->getCertificateBundle();
@@ -282,8 +267,6 @@ class CertificateManager implements ICertificateManager {
 
 	/**
 	 * get mtime of ca-bundle shipped by Nextcloud
-	 *
-	 * @return int
 	 */
 	protected function getFilemtimeOfCaBundle(): int {
 		return filemtime(\OC::$SERVERROOT . '/resources/config/ca-bundle.crt');

lib/private/Security/CredentialsManager.php

@@ -40,26 +40,16 @@ use OCP\Security\ICrypto;
 class CredentialsManager implements ICredentialsManager {
 	public const DB_TABLE = 'storages_credentials';
 
-	/** @var ICrypto */
-	protected $crypto;
-
-	/** @var IDBConnection */
-	protected $dbConnection;
-
-	/**
-	 * @param ICrypto $crypto
-	 * @param IDBConnection $dbConnection
-	 */
-	public function __construct(ICrypto $crypto, IDBConnection $dbConnection) {
-		$this->crypto = $crypto;
-		$this->dbConnection = $dbConnection;
+	public function __construct(
+		protected ICrypto $crypto,
+		protected IDBConnection $dbConnection,
+	) {
 	}
 
 	/**
 	 * Store a set of credentials
 	 *
 	 * @param string $userId empty string for system-wide credentials
-	 * @param string $identifier
 	 * @param mixed $credentials
 	 */
 	public function store(string $userId, string $identifier, $credentials): void {
@@ -77,10 +67,8 @@ class CredentialsManager implements ICredentialsManager {
 	 * Retrieve a set of credentials
 	 *
 	 * @param string $userId empty string for system-wide credentials
-	 * @param string $identifier
-	 * @return mixed
 	 */
-	public function retrieve(string $userId, string $identifier) {
+	public function retrieve(string $userId, string $identifier): mixed {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->select('credentials')
 			->from(self::DB_TABLE)
@@ -108,7 +96,6 @@ class CredentialsManager implements ICredentialsManager {
 	 * Delete a set of credentials
 	 *
 	 * @param string $userId empty string for system-wide credentials
-	 * @param string $identifier
 	 * @return int rows removed
 	 */
 	public function delete(string $userId, string $identifier): int {
@@ -128,7 +115,6 @@ class CredentialsManager implements ICredentialsManager {
 	/**
 	 * Erase all credentials stored for a user
 	 *
-	 * @param string $userId
 	 * @return int rows removed
 	 */
 	public function erase(string $userId): int {

lib/private/Security/Crypto.php

@@ -32,7 +32,6 @@ namespace OC\Security;
 use Exception;
 use OCP\IConfig;
 use OCP\Security\ICrypto;
-use OCP\Security\ISecureRandom;
 use phpseclib\Crypt\AES;
 use phpseclib\Crypt\Hash;
 
@@ -47,20 +46,13 @@ use phpseclib\Crypt\Hash;
  * @package OC\Security
  */
 class Crypto implements ICrypto {
-	/** @var AES $cipher */
-	private $cipher;
-	/** @var int */
-	private $ivLength = 16;
-	/** @var IConfig */
-	private $config;
+	private AES $cipher;
+	private int $ivLength = 16;
 
-	/**
-	 * @param IConfig $config
-	 * @param ISecureRandom $random
-	 */
-	public function __construct(IConfig $config) {
+	public function __construct(
+		private IConfig $config,
+	) {
 		$this->cipher = new AES();
-		$this->config = $config;
 	}
 
 	/**
@@ -84,7 +76,6 @@ class Crypto implements ICrypto {
 	/**
 	 * Encrypts a value and adds an HMAC (Encrypt-Then-MAC)
 	 *
-	 * @param string $plaintext
 	 * @param string $password Password to encrypt, if not specified the secret from config.php will be taken
 	 * @return string Authenticated ciphertext
 	 * @throws Exception if it was not possible to gather sufficient entropy
@@ -115,9 +106,7 @@ class Crypto implements ICrypto {
 
 	/**
 	 * Decrypts a value and verifies the HMAC (Encrypt-Then-Mac)
-	 * @param string $authenticatedCiphertext
 	 * @param string $password Password to encrypt, if not specified the secret from config.php will be taken
-	 * @return string plaintext
 	 * @throws Exception If the HMAC does not match
 	 * @throws Exception If the decryption failed
 	 */

lib/private/Security/Hasher.php

@@ -51,19 +51,14 @@ use OCP\Security\IHasher;
  * @package OC\Security
  */
 class Hasher implements IHasher {
-	/** @var IConfig */
-	private $config;
-	/** @var array Options passed to password_hash and password_needs_rehash */
-	private $options = [];
-	/** @var string Salt used for legacy passwords */
-	private $legacySalt = null;
-
-	/**
-	 * @param IConfig $config
-	 */
-	public function __construct(IConfig $config) {
-		$this->config = $config;
-
+	/** Options passed to password_hash and password_needs_rehash */
+	private array $options = [];
+	/** Salt used for legacy passwords */
+	private ?string $legacySalt = null;
+
+	public function __construct(
+		private IConfig $config,
+	) {
 		if (\defined('PASSWORD_ARGON2ID') || \defined('PASSWORD_ARGON2I')) {
 			// password_hash fails, when the minimum values are undershot.
 			// In this case, apply minimum.
@@ -106,7 +101,7 @@ class Hasher implements IHasher {
 	 * @param string $prefixedHash
 	 * @return null|array Null if the hash is not prefixed, otherwise array('version' => 1, 'hash' => 'foo')
 	 */
-	protected function splitHash(string $prefixedHash) {
+	protected function splitHash(string $prefixedHash): ?array {
 		$explodedString = explode('|', $prefixedHash, 2);
 		if (\count($explodedString) === 2) {
 			if ((int)$explodedString[0] > 0) {
@@ -198,7 +193,7 @@ class Hasher implements IHasher {
 		return password_needs_rehash($hash, $algorithm, $this->options);
 	}
 
-	private function getPrefferedAlgorithm() {
+	private function getPrefferedAlgorithm(): string {
 		$default = PASSWORD_BCRYPT;
 		if (\defined('PASSWORD_ARGON2I')) {
 			$default = PASSWORD_ARGON2I;

lib/private/Security/Normalizer/IpAddress.php

@@ -37,22 +37,16 @@ namespace OC\Security\Normalizer;
  * @package OC\Security\Normalizer
  */
 class IpAddress {
-	/** @var string */
-	private $ip;
-
 	/**
 	 * @param string $ip IP to normalized
 	 */
-	public function __construct(string $ip) {
-		$this->ip = $ip;
+	public function __construct(
+		private string $ip,
+	) {
 	}
 
 	/**
 	 * Return the given subnet for an IPv4 address and mask bits
-	 *
-	 * @param string $ip
-	 * @param int $maskBits
-	 * @return string
 	 */
 	private function getIPv4Subnet(string $ip, int $maskBits = 32): string {
 		$binary = \inet_pton($ip);
@@ -68,10 +62,6 @@ class IpAddress {
 
 	/**
 	 * Return the given subnet for an IPv6 address and mask bits
-	 *
-	 * @param string $ip
-	 * @param int $maskBits
-	 * @return string
 	 */
 	private function getIPv6Subnet(string $ip, int $maskBits = 48): string {
 		if ($ip[0] === '[' && $ip[-1] === ']') { // If IP is with brackets, for example [::1]
@@ -126,8 +116,6 @@ class IpAddress {
 
 	/**
 	 * Gets either the /32 (IPv4) or the /64 (IPv6) subnet of an IP address
-	 *
-	 * @return string
 	 */
 	public function getSubnet(): string {
 		if (\preg_match('/^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$/', $this->ip)) {
@@ -153,8 +141,6 @@ class IpAddress {
 
 	/**
 	 * Returns the specified IP address
-	 *
-	 * @return string
 	 */
 	public function __toString(): string {
 		return $this->ip;

lib/private/Security/RemoteHostValidator.php

@@ -38,19 +38,12 @@ use function urldecode;
  * @internal
  */
 final class RemoteHostValidator implements IRemoteHostValidator {
-	private IConfig $config;
-	private HostnameClassifier $hostnameClassifier;
-	private IpAddressClassifier $ipAddressClassifier;
-	private LoggerInterface $logger;
-
-	public function __construct(IConfig $config,
-								HostnameClassifier $hostnameClassifier,
-								IpAddressClassifier $ipAddressClassifier,
-								LoggerInterface $logger) {
-		$this->config = $config;
-		$this->hostnameClassifier = $hostnameClassifier;
-		$this->ipAddressClassifier = $ipAddressClassifier;
-		$this->logger = $logger;
+	public function __construct(
+		private IConfig $config,
+		private HostnameClassifier $hostnameClassifier,
+		private IpAddressClassifier $ipAddressClassifier,
+		private LoggerInterface $logger,
+	) {
 	}
 
 	public function isValid(string $host): bool {

lib/private/Security/SecureRandom.php

@@ -44,11 +44,12 @@ class SecureRandom implements ISecureRandom {
 	 * @param int $length The length of the generated string
 	 * @param string $characters An optional list of characters to use if no character list is
 	 * 							specified all valid base64 characters are used.
-	 * @return string
 	 * @throws \LengthException if an invalid length is requested
 	 */
-	public function generate(int $length,
-							 string $characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'): string {
+	public function generate(
+		int $length,
+		string $characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/',
+	): string {
 		if ($length <= 0) {
 			throw new \LengthException('Invalid length specified: ' . $length . ' must be bigger than 0');
 		}

lib/private/Security/TrustedDomainHelper.php

@@ -34,19 +34,13 @@ use OCP\IConfig;
 use OCP\Security\ITrustedDomainHelper;
 
 class TrustedDomainHelper implements ITrustedDomainHelper {
-	/** @var IConfig */
-	private $config;
-
-	/**
-	 * @param IConfig $config
-	 */
-	public function __construct(IConfig $config) {
-		$this->config = $config;
+	public function __construct(
+		private IConfig $config,
+	) {
 	}
 
 	/**
 	 * Strips a potential port from a domain (in format domain:port)
-	 * @param string $host
 	 * @return string $host without appended port
 	 */
 	private function getDomainWithoutPort(string $host): string {