Skip to content

Add a getAttributes to IControllerMethodReflector #37279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nickvergessen wants to merge 1 commit into from
Closed

Add a getAttributes to IControllerMethodReflector #37279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 4 additions & 12 deletions lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
use OCP\IRequest;
use OCP\Security\Bruteforce\MaxDelayReached;
use Psr\Log\LoggerInterface;
use ReflectionMethod;

/**
* Class BruteForceMiddleware performs the bruteforce protection for controllers
Expand Down Expand Up @@ -69,15 +68,11 @@ public function beforeController($controller, $methodName) {
$action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action');
$this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), $action);
} else {
$reflectionMethod = new ReflectionMethod($controller, $methodName);
$attributes = $reflectionMethod->getAttributes(BruteForceProtection::class);
$attributes = $this->reflector->getAttributes(BruteForceProtection::class);

Check failure

Code scanning / Psalm

InvalidArgument

Argument 1 of OC\AppFramework\Utility\ControllerMethodReflector::getAttributes expects class-string<Attribute>, but OCP\AppFramework\Http\Attribute\BruteForceProtection::class provided

if (!empty($attributes)) {
$remoteAddress = $this->request->getRemoteAddress();

foreach ($attributes as $attribute) {
/** @var BruteForceProtection $protection */
$protection = $attribute->newInstance();
foreach ($attributes as $protection) {
$action = $protection->getAction();
$this->throttler->sleepDelayOrThrowOnMax($remoteAddress, $action);
}
Expand All @@ -96,16 +91,13 @@ public function afterController($controller, $methodName, Response $response) {
$this->throttler->sleepDelay($ip, $action);
$this->throttler->registerAttempt($action, $ip, $response->getThrottleMetadata());
} else {
$reflectionMethod = new ReflectionMethod($controller, $methodName);
$attributes = $reflectionMethod->getAttributes(BruteForceProtection::class);
$attributes = $this->reflector->getAttributes(BruteForceProtection::class);

Check failure

Code scanning / Psalm

InvalidArgument

Argument 1 of OC\AppFramework\Utility\ControllerMethodReflector::getAttributes expects class-string<Attribute>, but OCP\AppFramework\Http\Attribute\BruteForceProtection::class provided

if (!empty($attributes)) {
$ip = $this->request->getRemoteAddress();
$metaData = $response->getThrottleMetadata();

foreach ($attributes as $attribute) {
/** @var BruteForceProtection $protection */
$protection = $attribute->newInstance();
foreach ($attributes as $protection) {
$action = $protection->getAction();

if (!isset($metaData['action']) || $metaData['action'] === $action) {
Expand Down
7 changes: 2 additions & 5 deletions lib/private/AppFramework/Middleware/SessionMiddleware.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Middleware;
use OCP\ISession;
use ReflectionMethod;

class SessionMiddleware extends Middleware {
/** @var ControllerMethodReflector */
Expand Down Expand Up @@ -63,8 +62,7 @@ public function beforeController($controller, $methodName) {
return;
}

$reflectionMethod = new ReflectionMethod($controller, $methodName);
$hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class));
$hasAttribute = !empty($this->reflector->getAttributes(UseSession::class));

Check failure

Code scanning / Psalm

InvalidArgument

Argument 1 of OC\AppFramework\Utility\ControllerMethodReflector::getAttributes expects class-string<Attribute>, but OCP\AppFramework\Http\Attribute\UseSession::class provided
if ($hasAttribute) {
$this->session->reopen();
}
Expand All @@ -86,8 +84,7 @@ public function afterController($controller, $methodName, Response $response) {
return $response;
}

$reflectionMethod = new ReflectionMethod($controller, $methodName);
$hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class));
$hasAttribute = !empty($this->reflector->getAttributes(UseSession::class));

Check failure

Code scanning / Psalm

InvalidArgument

Argument 1 of OC\AppFramework\Utility\ControllerMethodReflector::getAttributes expects class-string<Attribute>, but OCP\AppFramework\Http\Attribute\UseSession::class provided
if ($hasAttribute) {
$this->session->close();
}
Expand Down
23 changes: 20 additions & 3 deletions lib/private/AppFramework/Utility/ControllerMethodReflector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@
*/
namespace OC\AppFramework\Utility;

use Attribute;
use OCP\AppFramework\Utility\IControllerMethodReflector;

/**
* Reads and parses annotations from doc comments
*/
class ControllerMethodReflector implements IControllerMethodReflector {

protected ?\ReflectionMethod $reflection;
Copy link

@vitormattos vitormattos Mar 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more specific because exist other types of reflections:

Suggested change
protected ?\ReflectionMethod $reflection;
protected ?\ReflectionMethod $reflectionMethod;

public $annotations = [];
private $types = [];
private $parameters = [];
Expand All @@ -48,8 +51,8 @@ class ControllerMethodReflector implements IControllerMethodReflector {
* @param string $method the method which we want to inspect
*/
public function reflect($object, string $method) {
$reflection = new \ReflectionMethod($object, $method);
$docs = $reflection->getDocComment();
$this->reflection = new \ReflectionMethod($object, $method);
$docs = $this->reflection->getDocComment();

if ($docs !== false) {
// extract everything prefixed by @ and first letter uppercase
Expand All @@ -76,7 +79,7 @@ public function reflect($object, string $method) {
$this->types = array_combine($matches['var'], $matches['type']);
}

foreach ($reflection->getParameters() as $param) {
foreach ($this->reflection->getParameters() as $param) {
// extract type information from PHP 7 scalar types and prefer them over phpdoc annotations
$type = $param->getType();
if ($type instanceof \ReflectionNamedType) {
Expand Down Expand Up @@ -138,4 +141,18 @@ public function getAnnotationParameter(string $name, string $key): string {

return '';
}

/**
* @template T of Attribute
Copy link
Contributor

@come-nc come-nc Mar 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attribute classes do not extend Attribute class, which is final. So you cannot do that.

* @param class-string<T> $class
* @return T[]
*/
public function getAttributes(string $class): array {
$attributes = $this->reflection->getAttributes($class);
$return = [];
foreach ($attributes as $attribute) {
$return[] = $attribute->newInstance();
}
return $return;
}
}
10 changes: 10 additions & 0 deletions lib/public/AppFramework/Utility/IControllerMethodReflector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
*/
namespace OCP\AppFramework\Utility;

use Attribute;

/**
* Interface ControllerMethodReflector
*
Expand Down Expand Up @@ -75,4 +77,12 @@ public function getParameters(): array;
* @see https://help.nextcloud.com/t/how-should-we-use-php8-attributes/104278
*/
public function hasAnnotation(string $name): bool;

/**
* @template T of Attribute
* @param class-string<T> $class
* @return T[]
* @since 27.0.0
*/
public function getAttributes(string $class): array;
}