Merge pull request nextcloud#39011 from fsamapoor/refactor_lib_privat…

…e_security_part1

[1/3] Refactors lib/private/Security

lib/private/Security/Bruteforce/CleanupJob.php

@@ -32,19 +32,18 @@
 use OCP\IDBConnection;
 
 class CleanupJob extends TimedJob {
-	/** @var IDBConnection */
-	private $connection;
-
-	public function __construct(ITimeFactory $time, IDBConnection $connection) {
+	public function __construct(
+		ITimeFactory $time,
+		private IDBConnection $connection,
+	) {
 		parent::__construct($time);
-		$this->connection = $connection;
 
 		// Run once a day
 		$this->setInterval(3600 * 24);
 		$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
 	}
 
-	protected function run($argument) {
+	protected function run($argument): void {
 		// Delete all entries more than 48 hours old
 		$time = $this->time->getTime() - (48 * 3600);
 

lib/private/Security/Bruteforce/Throttler.php

@@ -106,9 +106,6 @@ public function registerAttempt(string $action,
 
 	/**
 	 * Check if the IP is whitelisted
-	 *
-	 * @param string $ip
-	 * @return bool
 	 */
 	public function isBypassListed(string $ip): bool {
 		if (isset($this->ipIsWhitelisted[$ip])) {

lib/private/Security/CSP/ContentSecurityPolicy.php

@@ -34,33 +34,22 @@
  * @package OC\Security\CSP
  */
 class ContentSecurityPolicy extends \OCP\AppFramework\Http\ContentSecurityPolicy {
-	/**
-	 * @return boolean
-	 */
 	public function isInlineScriptAllowed(): bool {
 		return $this->inlineScriptAllowed;
 	}
 
-	/**
-	 * @param boolean $inlineScriptAllowed
-	 */
-	public function setInlineScriptAllowed(bool $inlineScriptAllowed) {
+	public function setInlineScriptAllowed(bool $inlineScriptAllowed): void {
 		$this->inlineScriptAllowed = $inlineScriptAllowed;
 	}
 
-	/**
-	 * @return boolean
-	 */
 	public function isEvalScriptAllowed(): bool {
 		return $this->evalScriptAllowed;
 	}
 
 	/**
-	 * @param boolean $evalScriptAllowed
-	 *
 	 * @deprecated 17.0.0 Unsafe eval should not be used anymore.
 	 */
-	public function setEvalScriptAllowed(bool $evalScriptAllowed) {
+	public function setEvalScriptAllowed(bool $evalScriptAllowed): void {
 		$this->evalScriptAllowed = $evalScriptAllowed;
 	}
 
@@ -72,134 +61,79 @@ public function setEvalWasmAllowed(bool $evalWasmAllowed): void {
 		$this->evalWasmAllowed = $evalWasmAllowed;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedScriptDomains(): array {
 		return $this->allowedScriptDomains;
 	}
 
-	/**
-	 * @param array $allowedScriptDomains
-	 */
-	public function setAllowedScriptDomains(array $allowedScriptDomains) {
+	public function setAllowedScriptDomains(array $allowedScriptDomains): void {
 		$this->allowedScriptDomains = $allowedScriptDomains;
 	}
 
-	/**
-	 * @return boolean
-	 */
 	public function isInlineStyleAllowed(): bool {
 		return $this->inlineStyleAllowed;
 	}
 
-	/**
-	 * @param boolean $inlineStyleAllowed
-	 */
-	public function setInlineStyleAllowed(bool $inlineStyleAllowed) {
+	public function setInlineStyleAllowed(bool $inlineStyleAllowed): void {
 		$this->inlineStyleAllowed = $inlineStyleAllowed;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedStyleDomains(): array {
 		return $this->allowedStyleDomains;
 	}
 
-	/**
-	 * @param array $allowedStyleDomains
-	 */
-	public function setAllowedStyleDomains(array $allowedStyleDomains) {
+	public function setAllowedStyleDomains(array $allowedStyleDomains): void {
 		$this->allowedStyleDomains = $allowedStyleDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedImageDomains(): array {
 		return $this->allowedImageDomains;
 	}
 
-	/**
-	 * @param array $allowedImageDomains
-	 */
-	public function setAllowedImageDomains(array $allowedImageDomains) {
+	public function setAllowedImageDomains(array $allowedImageDomains): void {
 		$this->allowedImageDomains = $allowedImageDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedConnectDomains(): array {
 		return $this->allowedConnectDomains;
 	}
 
-	/**
-	 * @param array $allowedConnectDomains
-	 */
-	public function setAllowedConnectDomains(array $allowedConnectDomains) {
+	public function setAllowedConnectDomains(array $allowedConnectDomains): void {
 		$this->allowedConnectDomains = $allowedConnectDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedMediaDomains(): array {
 		return $this->allowedMediaDomains;
 	}
 
-	/**
-	 * @param array $allowedMediaDomains
-	 */
-	public function setAllowedMediaDomains(array $allowedMediaDomains) {
+	public function setAllowedMediaDomains(array $allowedMediaDomains): void {
 		$this->allowedMediaDomains = $allowedMediaDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedObjectDomains(): array {
 		return $this->allowedObjectDomains;
 	}
 
-	/**
-	 * @param array $allowedObjectDomains
-	 */
-	public function setAllowedObjectDomains(array $allowedObjectDomains) {
+	public function setAllowedObjectDomains(array $allowedObjectDomains): void {
 		$this->allowedObjectDomains = $allowedObjectDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedFrameDomains(): array {
 		return $this->allowedFrameDomains;
 	}
 
-	/**
-	 * @param array $allowedFrameDomains
-	 */
-	public function setAllowedFrameDomains(array $allowedFrameDomains) {
+	public function setAllowedFrameDomains(array $allowedFrameDomains): void {
 		$this->allowedFrameDomains = $allowedFrameDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedFontDomains(): array {
 		return $this->allowedFontDomains;
 	}
 
-	/**
-	 * @param array $allowedFontDomains
-	 */
-	public function setAllowedFontDomains($allowedFontDomains) {
+	public function setAllowedFontDomains($allowedFontDomains): void {
 		$this->allowedFontDomains = $allowedFontDomains;
 	}
 
 	/**
-	 * @return array
 	 * @deprecated 15.0.0 use FrameDomains and WorkerSrcDomains
 	 */
 	public function getAllowedChildSrcDomains(): array {
@@ -210,29 +144,26 @@ public function getAllowedChildSrcDomains(): array {
 	 * @param array $allowedChildSrcDomains
 	 * @deprecated 15.0.0 use FrameDomains and WorkerSrcDomains
 	 */
-	public function setAllowedChildSrcDomains($allowedChildSrcDomains) {
+	public function setAllowedChildSrcDomains($allowedChildSrcDomains): void {
 		$this->allowedChildSrcDomains = $allowedChildSrcDomains;
 	}
 
-	/**
-	 * @return array
-	 */
 	public function getAllowedFrameAncestors(): array {
 		return $this->allowedFrameAncestors;
 	}
 
 	/**
 	 * @param array $allowedFrameAncestors
 	 */
-	public function setAllowedFrameAncestors($allowedFrameAncestors) {
+	public function setAllowedFrameAncestors($allowedFrameAncestors): void {
 		$this->allowedFrameAncestors = $allowedFrameAncestors;
 	}
 
 	public function getAllowedWorkerSrcDomains(): array {
 		return $this->allowedWorkerSrcDomains;
 	}
 
-	public function setAllowedWorkerSrcDomains(array $allowedWorkerSrcDomains) {
+	public function setAllowedWorkerSrcDomains(array $allowedWorkerSrcDomains): void {
 		$this->allowedWorkerSrcDomains = $allowedWorkerSrcDomains;
 	}
 
@@ -249,21 +180,15 @@ public function getReportTo(): array {
 		return $this->reportTo;
 	}
 
-	public function setReportTo(array $reportTo) {
+	public function setReportTo(array $reportTo): void {
 		$this->reportTo = $reportTo;
 	}
 
-	/**
-	 * @return boolean
-	 */
 	public function isStrictDynamicAllowed(): bool {
 		return $this->strictDynamicAllowed;
 	}
 
-	/**
-	 * @param boolean $strictDynamicAllowed
-	 */
-	public function setStrictDynamicAllowed(bool $strictDynamicAllowed) {
+	public function setStrictDynamicAllowed(bool $strictDynamicAllowed): void {
 		$this->strictDynamicAllowed = $strictDynamicAllowed;
 	}
 }

lib/private/Security/CSP/ContentSecurityPolicyManager.php

@@ -35,25 +35,21 @@
 
 class ContentSecurityPolicyManager implements IContentSecurityPolicyManager {
 	/** @var ContentSecurityPolicy[] */
-	private $policies = [];
+	private array $policies = [];
 
-	/** @var IEventDispatcher */
-	private $dispatcher;
-
-	public function __construct(IEventDispatcher $dispatcher) {
-		$this->dispatcher = $dispatcher;
+	public function __construct(
+		private IEventDispatcher $dispatcher,
+	) {
 	}
 
 	/** {@inheritdoc} */
-	public function addDefaultPolicy(EmptyContentSecurityPolicy $policy) {
+	public function addDefaultPolicy(EmptyContentSecurityPolicy $policy): void {
 		$this->policies[] = $policy;
 	}
 
 	/**
 	 * Get the configured default policy. This is not in the public namespace
 	 * as it is only supposed to be used by core itself.
-	 *
-	 * @return ContentSecurityPolicy
 	 */
 	public function getDefaultPolicy(): ContentSecurityPolicy {
 		$event = new AddContentSecurityPolicyEvent($this);
@@ -68,13 +64,11 @@ public function getDefaultPolicy(): ContentSecurityPolicy {
 
 	/**
 	 * Merges the first given policy with the second one
-	 *
-	 * @param ContentSecurityPolicy $defaultPolicy
-	 * @param EmptyContentSecurityPolicy $originalPolicy
-	 * @return ContentSecurityPolicy
 	 */
-	public function mergePolicies(ContentSecurityPolicy $defaultPolicy,
-								  EmptyContentSecurityPolicy $originalPolicy): ContentSecurityPolicy {
+	public function mergePolicies(
+		ContentSecurityPolicy $defaultPolicy,
+		EmptyContentSecurityPolicy $originalPolicy,
+	): ContentSecurityPolicy {
 		foreach ((object)(array)$originalPolicy as $name => $value) {
 			$setter = 'set'.ucfirst($name);
 			if (\is_array($value)) {

lib/private/Security/CSP/ContentSecurityPolicyNonceManager.php

@@ -38,27 +38,16 @@
  * @package OC\Security\CSP
  */
 class ContentSecurityPolicyNonceManager {
-	/** @var CsrfTokenManager */
-	private $csrfTokenManager;
-	/** @var IRequest */
-	private $request;
-	/** @var string */
-	private $nonce = '';
+	private string $nonce = '';
 
-	/**
-	 * @param CsrfTokenManager $csrfTokenManager
-	 * @param IRequest $request
-	 */
-	public function __construct(CsrfTokenManager $csrfTokenManager,
-								IRequest $request) {
-		$this->csrfTokenManager = $csrfTokenManager;
-		$this->request = $request;
+	public function __construct(
+		private CsrfTokenManager $csrfTokenManager,
+		private IRequest $request,
+	) {
 	}
 
 	/**
-	 * Returns the current CSP nounce
-	 *
-	 * @return string
+	 * Returns the current CSP nonce
 	 */
 	public function getNonce(): string {
 		if ($this->nonce === '') {
@@ -74,8 +63,6 @@ public function getNonce(): string {
 
 	/**
 	 * Check if the browser supports CSP v3
-	 *
-	 * @return bool
 	 */
 	public function browserSupportsCspV3(): bool {
 		$browserWhitelist = [