Merge pull request #26626 from J0WI/strict-security

Make Security module strict

lib/private/Security/Bruteforce/Capabilities.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Roeland Jago Douma <roeland@famdouma.nl>
  *
@@ -46,7 +49,7 @@ class Capabilities implements IPublicCapability {
 		$this->throttler = $throttler;
 	}
 
-	public function getCapabilities() {
+	public function getCapabilities(): array {
 		if (version_compare(\OC::$server->getConfig()->getSystemValue('version', '0.0.0.0'), '12.0.0.0', '<')) {
 			return [];
 		}

lib/private/Security/Certificate.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -49,7 +52,7 @@ class Certificate implements ICertificate {
 	 * @param string $name
 	 * @throws \Exception If the certificate could not get parsed
 	 */
-	public function __construct($data, $name) {
+	public function __construct(string $data, string $name) {
 		$this->name = $name;
 		$gmt = new \DateTimeZone('GMT');
 
@@ -75,42 +78,42 @@ class Certificate implements ICertificate {
 	/**
 	 * @return string
 	 */
-	public function getName() {
+	public function getName(): string {
 		return $this->name;
 	}
 
 	/**
 	 * @return string|null
 	 */
-	public function getCommonName() {
+	public function getCommonName(): ?string {
 		return $this->commonName;
 	}
 
 	/**
-	 * @return string
+	 * @return string|null
 	 */
-	public function getOrganization() {
+	public function getOrganization(): ?string {
 		return $this->organization;
 	}
 
 	/**
 	 * @return \DateTime
 	 */
-	public function getIssueDate() {
+	public function getIssueDate(): \DateTime {
 		return $this->issueDate;
 	}
 
 	/**
 	 * @return \DateTime
 	 */
-	public function getExpireDate() {
+	public function getExpireDate(): \DateTime {
 		return $this->expireDate;
 	}
 
 	/**
 	 * @return bool
 	 */
-	public function isExpired() {
+	public function isExpired(): bool {
 		$now = new \DateTime();
 		return $this->issueDate > $now or $now > $this->expireDate;
 	}
@@ -118,14 +121,14 @@ class Certificate implements ICertificate {
 	/**
 	 * @return string|null
 	 */
-	public function getIssuerName() {
+	public function getIssuerName(): ?string {
 		return $this->issuerName;
 	}
 
 	/**
 	 * @return string|null
 	 */
-	public function getIssuerOrganization() {
+	public function getIssuerOrganization(): ?string {
 		return $this->issuerOrganization;
 	}
 }

lib/private/Security/CertificateManager.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -30,6 +33,7 @@
 namespace OC\Security;
 
 use OC\Files\Filesystem;
+use OCP\ICertificate;
 use OCP\ICertificateManager;
 use OCP\IConfig;
 use OCP\ILogger;
@@ -78,7 +82,7 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @return \OCP\ICertificate[]
 	 */
-	public function listCertificates() {
+	public function listCertificates(): array {
 		if (!$this->config->getSystemValue('installed', false)) {
 			return [];
 		}
@@ -130,7 +134,7 @@ class CertificateManager implements ICertificateManager {
 	/**
 	 * create the certificate bundle of all trusted certificated
 	 */
-	public function createCertificateBundle() {
+	public function createCertificateBundle(): void {
 		$path = $this->getPathToCertificates();
 		$certs = $this->listCertificates();
 
@@ -182,7 +186,7 @@ class CertificateManager implements ICertificateManager {
 	 * @return \OCP\ICertificate
 	 * @throws \Exception If the certificate could not get added
 	 */
-	public function addCertificate($certificate, $name) {
+	public function addCertificate(string $certificate, string $name): ICertificate {
 		if (!Filesystem::isValidPath($name) or Filesystem::isFileBlacklisted($name)) {
 			throw new \Exception('Filename is not valid');
 		}
@@ -209,7 +213,7 @@ class CertificateManager implements ICertificateManager {
 	 * @param string $name
 	 * @return bool
 	 */
-	public function removeCertificate($name) {
+	public function removeCertificate(string $name): bool {
 		if (!Filesystem::isValidPath($name)) {
 			return false;
 		}
@@ -226,7 +230,7 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @return string
 	 */
-	public function getCertificateBundle() {
+	public function getCertificateBundle(): string {
 		return $this->getPathToCertificates() . 'rootcerts.crt';
 	}
 
@@ -235,7 +239,7 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @return string
 	 */
-	public function getAbsoluteBundlePath() {
+	public function getAbsoluteBundlePath(): string {
 		if (!$this->hasCertificates()) {
 			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
 		}
@@ -250,7 +254,7 @@ class CertificateManager implements ICertificateManager {
 	/**
 	 * @return string
 	 */
-	private function getPathToCertificates() {
+	private function getPathToCertificates(): string {
 		return '/files_external/';
 	}
 
@@ -259,7 +263,7 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @return bool
 	 */
-	private function needsRebundling() {
+	private function needsRebundling(): bool {
 		$targetBundle = $this->getCertificateBundle();
 		if (!$this->view->file_exists($targetBundle)) {
 			return true;
@@ -274,7 +278,7 @@ class CertificateManager implements ICertificateManager {
 	 *
 	 * @return int
 	 */
-	protected function getFilemtimeOfCaBundle() {
+	protected function getFilemtimeOfCaBundle(): int {
 		return filemtime(\OC::$SERVERROOT . '/resources/config/ca-bundle.crt');
 	}
 }

lib/private/Security/CredentialsManager.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -59,11 +62,11 @@ class CredentialsManager implements ICredentialsManager {
 	 * @param string $identifier
 	 * @param mixed $credentials
 	 */
-	public function store($userId, $identifier, $credentials) {
+	public function store(string $userId, string $identifier, $credentials): void {
 		$value = $this->crypto->encrypt(json_encode($credentials));
 
 		$this->dbConnection->setValues(self::DB_TABLE, [
-			'user' => (string)$userId,
+			'user' => $userId,
 			'identifier' => $identifier,
 		], [
 			'credentials' => $value,
@@ -77,7 +80,7 @@ class CredentialsManager implements ICredentialsManager {
 	 * @param string $identifier
 	 * @return mixed
 	 */
-	public function retrieve($userId, $identifier) {
+	public function retrieve(string $userId, string $identifier) {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->select('credentials')
 			->from(self::DB_TABLE)
@@ -86,7 +89,7 @@ class CredentialsManager implements ICredentialsManager {
 		if ($userId === '') {
 			$qb->andWhere($qb->expr()->emptyString('user'));
 		} else {
-			$qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter((string)$userId)));
+			$qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($userId)));
 		}
 
 		$qResult = $qb->execute();
@@ -108,7 +111,7 @@ class CredentialsManager implements ICredentialsManager {
 	 * @param string $identifier
 	 * @return int rows removed
 	 */
-	public function delete($userId, $identifier) {
+	public function delete(string $userId, string $identifier): int {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->delete(self::DB_TABLE)
 			->where($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier)));
@@ -116,7 +119,7 @@ class CredentialsManager implements ICredentialsManager {
 		if ($userId === '') {
 			$qb->andWhere($qb->expr()->emptyString('user'));
 		} else {
-			$qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter((string)$userId)));
+			$qb->andWhere($qb->expr()->eq('user', $qb->createNamedParameter($userId)));
 		}
 
 		return $qb->execute();
@@ -128,7 +131,7 @@ class CredentialsManager implements ICredentialsManager {
 	 * @param string $userId
 	 * @return int rows removed
 	 */
-	public function erase($userId) {
+	public function erase(string $userId): int {
 		$qb = $this->dbConnection->getQueryBuilder();
 		$qb->delete(self::DB_TABLE)
 			->where($qb->expr()->eq('user', $qb->createNamedParameter($userId)))

lib/private/Security/TrustedDomainHelper.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -51,7 +54,7 @@ class TrustedDomainHelper {
 	 * @param string $host
 	 * @return string $host without appended port
 	 */
-	private function getDomainWithoutPort($host) {
+	private function getDomainWithoutPort(string $host): string {
 		$pos = strrpos($host, ':');
 		if ($pos !== false) {
 			$port = substr($host, $pos + 1);
@@ -71,7 +74,7 @@ class TrustedDomainHelper {
 	 * @return bool true if the given domain is trusted or if no trusted domains
 	 * have been configured
 	 */
-	public function isTrustedDomain($domainWithPort) {
+	public function isTrustedDomain(string $domainWithPort): bool {
 		// overwritehost is always trusted
 		if ($this->config->getSystemValue('overwritehost') !== '') {
 			return true;

lib/public/ICertificate.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -33,47 +36,47 @@ interface ICertificate {
 	 * @return string
 	 * @since 8.0.0
 	 */
-	public function getName();
+	public function getName(): string;
 
 	/**
-	 * @return string
+	 * @return string|null
 	 * @since 8.0.0
 	 */
-	public function getCommonName();
+	public function getCommonName(): ?string;
 
 	/**
-	 * @return string
+	 * @return string|null
 	 * @since 8.0.0
 	 */
-	public function getOrganization();
+	public function getOrganization(): ?string;
 
 	/**
 	 * @return \DateTime
 	 * @since 8.0.0
 	 */
-	public function getIssueDate();
+	public function getIssueDate(): \DateTime;
 
 	/**
 	 * @return \DateTime
 	 * @since 8.0.0
 	 */
-	public function getExpireDate();
+	public function getExpireDate(): \DateTime;
 
 	/**
 	 * @return bool
 	 * @since 8.0.0
 	 */
-	public function isExpired();
+	public function isExpired(): bool;
 
 	/**
-	 * @return string
+	 * @return string|null
 	 * @since 8.0.0
 	 */
-	public function getIssuerName();
+	public function getIssuerName(): ?string;
 
 	/**
-	 * @return string
+	 * @return string|null
 	 * @since 8.0.0
 	 */
-	public function getIssuerOrganization();
+	public function getIssuerOrganization(): ?string;
 }

lib/public/ICertificateManager.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -35,7 +38,7 @@ interface ICertificateManager {
 	 * @return \OCP\ICertificate[]
 	 * @since 8.0.0
 	 */
-	public function listCertificates();
+	public function listCertificates(): array;
 
 	/**
 	 * @param string $certificate the certificate data
@@ -44,13 +47,14 @@ interface ICertificateManager {
 	 * @throws \Exception If the certificate could not get added
 	 * @since 8.0.0 - since 8.1.0 throws exception instead of returning false
 	 */
-	public function addCertificate($certificate, $name);
+	public function addCertificate(string $certificate, string $name): \OCP\ICertificate;
 
 	/**
 	 * @param string $name
+	 * @return bool
 	 * @since 8.0.0
 	 */
-	public function removeCertificate($name);
+	public function removeCertificate(string $name): bool;
 
 	/**
 	 * Get the path to the certificate bundle
@@ -58,7 +62,7 @@ interface ICertificateManager {
 	 * @return string
 	 * @since 8.0.0
 	 */
-	public function getCertificateBundle();
+	public function getCertificateBundle(): string;
 
 	/**
 	 * Get the full local path to the certificate bundle
@@ -66,5 +70,5 @@ interface ICertificateManager {
 	 * @return string
 	 * @since 9.0.0
 	 */
-	public function getAbsoluteBundlePath();
+	public function getAbsoluteBundlePath(): string;
 }

lib/public/Security/IContentSecurityPolicyManager.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *

lib/public/Security/ICredentialsManager.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -38,7 +41,7 @@ interface ICredentialsManager {
 	 * @param mixed $credentials
 	 * @since 8.2.0
 	 */
-	public function store($userId, $identifier, $credentials);
+	public function store(string $userId, string $identifier, $credentials): void;
 
 	/**
 	 * Retrieve a set of credentials
@@ -48,7 +51,7 @@ interface ICredentialsManager {
 	 * @return mixed
 	 * @since 8.2.0
 	 */
-	public function retrieve($userId, $identifier);
+	public function retrieve(string $userId, string $identifier);
 
 	/**
 	 * Delete a set of credentials
@@ -58,7 +61,7 @@ interface ICredentialsManager {
 	 * @return int rows removed
 	 * @since 8.2.0
 	 */
-	public function delete($userId, $identifier);
+	public function delete(string $userId, string $identifier): int;
 
 	/**
 	 * Erase all credentials stored for a user
@@ -67,5 +70,5 @@ interface ICredentialsManager {
 	 * @return int rows removed
 	 * @since 8.2.0
 	 */
-	public function erase($userId);
+	public function erase(string $userId): int;
 }

tests/lib/Security/Bruteforce/CapabilitiesTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Roeland Jago Douma <roeland@famdouma.nl>
  *

tests/lib/Security/Bruteforce/ThrottlerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016 Lukas Reschke <lukas@statuscode.ch>
  *

tests/lib/Security/CSP/AddContentSecurityPolicyEventTest.php

@@ -1,6 +1,7 @@
 <?php
 
 declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2019, Roeland Jago Douma <roeland@famdouma.nl>
  *

tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Lukas Reschke <lukas@owncloud.com>
  *

tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016 Lukas Reschke <lukas@statuscode.ch>
  *

tests/lib/Security/CSRF/CsrfTokenGeneratorTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Lukas Reschke <lukas@owncloud.com>
  *

tests/lib/Security/CSRF/CsrfTokenManagerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Lukas Reschke <lukas@owncloud.com>
  *

tests/lib/Security/CSRF/CsrfTokenTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Lukas Reschke <lukas@owncloud.com>
  *

tests/lib/Security/CSRF/TokenStorage/SessionStorageTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Lukas Reschke <lukas@owncloud.com>
  *

tests/lib/Security/CertificateManagerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or

tests/lib/Security/CertificateTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Lukas Reschke <lukas@owncloud.com>
  *

tests/lib/Security/CredentialsManagerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @author Robin McCorkell <rmccorkell@owncloud.com>
  *

tests/lib/Security/CryptoTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or

tests/lib/Security/HasherTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or

tests/lib/Security/IdentityProof/KeyTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright 2016, Roeland Jago Douma <roeland@famdouma.nl>
  *

tests/lib/Security/IdentityProof/ManagerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2016 Lukas Reschke <lukas@statuscode.ch>
  *

tests/lib/Security/IdentityProof/SignerTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright 2016, Roeland Jago Douma <roeland@famdouma.nl>
  *

tests/lib/Security/Normalizer/IpAddressTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
  *

tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
  *

tests/lib/Security/RateLimiting/LimiterTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
  *

tests/lib/Security/SecureRandomTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * Copyright (c) 2014 Lukas Reschke <lukas@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or

tests/lib/Security/TrustedDomainHelperTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * Copyright (c) 2015 Lukas Reschke <lukas@owncloud.com>
  * This file is licensed under the Affero General Public License version 3 or