Skip to content

Commit 1b504bf

Browse files
AltahrimAltahrim
authored
Merge pull request nextcloud#58863 from nextcloud/fix/annotation-attributes-fix
2 parents 22976d2 + e03d825 commit 1b504bf

22 files changed

+138
-168
lines changed

‎apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OCA\Provisioning_API\Middleware\Exceptions\NotSubAdminException;
1212
use OCP\AppFramework\Controller;
1313
use OCP\AppFramework\Http;
14+
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
1415
use OCP\AppFramework\Http\Response;
1516
use OCP\AppFramework\Middleware;
1617
use OCP\AppFramework\OCS\OCSException;
@@ -40,7 +41,7 @@ public function __construct(
4041
*/
4142
public function beforeController($controller, $methodName) {
4243
// If AuthorizedAdminSetting, the check will be done in the SecurityMiddleware
43-
if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) {
44+
if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotationOrAttribute('AuthorizedAdminSetting', AuthorizedAdminSetting::class)) {
4445
throw new NotSubAdminException();
4546
}
4647
}

‎apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,14 @@ public function testBeforeController(bool $subadminRequired, bool $isAdmin, bool
5353
);
5454

5555
$this->reflector->method('hasAnnotation')
56-
->willReturnCallback(function ($annotation) use ($subadminRequired, $hasSettingAuthorizationAnnotation) {
56+
->willReturnCallback(function ($annotation) use ($subadminRequired) {
5757
if ($annotation === 'NoSubAdminRequired') {
5858
return !$subadminRequired;
5959
}
60+
return false;
61+
});
62+
$this->reflector->method('hasAnnotationOrAttribute')
63+
->willReturnCallback(function ($annotation, $attribute) use ($hasSettingAuthorizationAnnotation) {
6064
if ($annotation === 'AuthorizedAdminSetting') {
6165
return $hasSettingAuthorizationAnnotation;
6266
}

‎apps/settings/lib/Middleware/SubadminMiddleware.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
1515
use OC\AppFramework\Utility\ControllerMethodReflector;
1616
use OCP\AppFramework\Controller;
17+
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
1718
use OCP\AppFramework\Http\TemplateResponse;
1819
use OCP\AppFramework\Middleware;
1920
use OCP\Group\ISubAdmin;
@@ -44,7 +45,7 @@ private function isSubAdmin(): bool {
4445

4546
#[Override]
4647
public function beforeController(Controller $controller, string $methodName): void {
47-
if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) {
48+
if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotationOrAttribute('AuthorizedAdminSetting', AuthorizedAdminSetting::class)) {
4849
if (!$this->isSubAdmin()) {
4950
throw new NotAdminException($this->l10n->t('Logged in account must be a sub admin'));
5051
}

‎apps/settings/tests/Middleware/SubadminMiddlewareTest.php‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OC\AppFramework\Utility\ControllerMethodReflector;
1515
use OCA\Settings\Middleware\SubadminMiddleware;
1616
use OCP\AppFramework\Controller;
17+
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
1718
use OCP\AppFramework\Http\TemplateResponse;
1819
use OCP\Group\ISubAdmin;
1920
use OCP\IL10N;
@@ -62,11 +63,16 @@ public function testBeforeControllerAsUserWithoutAnnotation(): void {
6263
$this->expectException(NotAdminException::class);
6364

6465
$this->reflector
65-
->expects($this->exactly(2))
66+
->expects($this->exactly(1))
6667
->method('hasAnnotation')
6768
->willReturnMap([
6869
['NoSubAdminRequired', false],
69-
['AuthorizedAdminSetting', false],
70+
]);
71+
$this->reflector
72+
->expects($this->exactly(1))
73+
->method('hasAnnotationOrAttribute')
74+
->willReturnMap([
75+
['AuthorizedAdminSetting', AuthorizedAdminSetting::class, false],
7076
]);
7177

7278
$this->subAdminManager
@@ -94,11 +100,16 @@ public function testBeforeControllerWithAnnotation(): void {
94100

95101
public function testBeforeControllerAsSubAdminWithoutAnnotation(): void {
96102
$this->reflector
97-
->expects($this->exactly(2))
103+
->expects($this->exactly(1))
98104
->method('hasAnnotation')
99105
->willReturnMap([
100106
['NoSubAdminRequired', false],
101-
['AuthorizedAdminSetting', false],
107+
]);
108+
$this->reflector
109+
->expects($this->exactly(1))
110+
->method('hasAnnotationOrAttribute')
111+
->willReturnMap([
112+
['AuthorizedAdminSetting', AuthorizedAdminSetting::class, false],
102113
]);
103114

104115
$this->subAdminManager

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,9 +1552,6 @@
15521552
</DeprecatedMethod>
15531553
</file>
15541554
<file src="apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php">
1555-
<DeprecatedInterface>
1556-
<code><![CDATA[protected]]></code>
1557-
</DeprecatedInterface>
15581555
<DeprecatedMethod>
15591556
<code><![CDATA[getAppValue]]></code>
15601557
<code><![CDATA[getAppValue]]></code>
@@ -2097,9 +2094,6 @@
20972094
</DeprecatedMethod>
20982095
</file>
20992096
<file src="apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php">
2100-
<DeprecatedInterface>
2101-
<code><![CDATA[IControllerMethodReflector]]></code>
2102-
</DeprecatedInterface>
21032097
<DeprecatedMethod>
21042098
<code><![CDATA[hasAnnotation]]></code>
21052099
<code><![CDATA[hasAnnotation]]></code>

‎core/Middleware/TwoFactorMiddleware.php‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
use Exception;
1313
use OC\AppFramework\Http\Attributes\TwoFactorSetUpDoneRequired;
14-
use OC\AppFramework\Middleware\MiddlewareUtils;
1514
use OC\Authentication\Exceptions\TwoFactorAuthRequiredException;
1615
use OC\Authentication\Exceptions\UserAlreadyLoggedInException;
1716
use OC\Authentication\TwoFactorAuth\Manager;
@@ -23,6 +22,7 @@
2322
use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired;
2423
use OCP\AppFramework\Http\RedirectResponse;
2524
use OCP\AppFramework\Middleware;
25+
use OCP\AppFramework\Utility\IControllerMethodReflector;
2626
use OCP\Authentication\TwoFactorAuth\ALoginSetupController;
2727
use OCP\IRequest;
2828
use OCP\ISession;
@@ -36,7 +36,7 @@ public function __construct(
3636
private Session $userSession,
3737
private ISession $session,
3838
private IURLGenerator $urlGenerator,
39-
private MiddlewareUtils $middlewareUtils,
39+
private IControllerMethodReflector $reflector,
4040
private IRequest $request,
4141
) {
4242
}
@@ -46,9 +46,7 @@ public function __construct(
4646
* @param string $methodName
4747
*/
4848
public function beforeController($controller, $methodName) {
49-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
50-
51-
if ($this->middlewareUtils->hasAnnotationOrAttribute($reflectionMethod, 'NoTwoFactorRequired', NoTwoFactorRequired::class)) {
49+
if ($this->reflector->hasAnnotationOrAttribute('NoTwoFactorRequired', NoTwoFactorRequired::class)) {
5250
// Route handler explicitly marked to work without finished 2FA are
5351
// not blocked
5452
return;
@@ -59,6 +57,7 @@ public function beforeController($controller, $methodName) {
5957
return;
6058
}
6159

60+
$reflectionMethod = new ReflectionMethod($controller, $methodName);
6261
if ($controller instanceof TwoFactorChallengeController
6362
&& $this->userSession->getUser() !== null
6463
&& !$reflectionMethod->getAttributes(TwoFactorSetUpDoneRequired::class)) {

‎lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use OCP\ISession;
1919
use OCP\IUserSession;
2020
use Psr\Log\LoggerInterface;
21-
use ReflectionMethod;
2221

2322
// Will close the session if the user session is ephemeral.
2423
// Happens when the user logs in via the login flow v2.
@@ -61,12 +60,7 @@ public function beforeController(Controller $controller, string $methodName) {
6160
return;
6261
}
6362

64-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
65-
if (!empty($reflectionMethod->getAttributes(PublicPage::class))) {
66-
return;
67-
}
68-
69-
if ($this->reflector->hasAnnotation('PublicPage')) {
63+
if ($this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class)) {
7064
return;
7165
}
7266

‎lib/private/AppFramework/Middleware/MiddlewareUtils.php‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,10 @@ public function __construct(
3030
* @param ReflectionMethod $reflectionMethod
3131
* @param ?string $annotationName
3232
* @param class-string<T> $attributeClass
33-
* @return boolean
33+
* @deprecated 34.0.0 call directly on the reflector
3434
*/
3535
public function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, ?string $annotationName, string $attributeClass): bool {
36-
if (!empty($reflectionMethod->getAttributes($attributeClass))) {
37-
return true;
38-
}
39-
40-
if ($annotationName && $this->reflector->hasAnnotation($annotationName)) {
41-
$this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead');
42-
return true;
43-
}
44-
45-
return false;
36+
return $this->reflector->hasAnnotationOrAttribute($annotationName, $attributeClass);
4637
}
4738

4839
/**

‎lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,7 @@ public function __construct(
4646
* @throws NotConfirmedException
4747
*/
4848
public function beforeController(Controller $controller, string $methodName) {
49-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
50-
51-
if (!$this->needsPasswordConfirmation($reflectionMethod)) {
49+
if (!$this->needsPasswordConfirmation()) {
5250
return;
5351
}
5452

@@ -79,6 +77,7 @@ public function beforeController(Controller $controller, string $methodName) {
7977
return;
8078
}
8179

80+
$reflectionMethod = new ReflectionMethod($controller, $methodName);
8281
if ($this->isPasswordConfirmationStrict($reflectionMethod)) {
8382
$authHeader = $this->request->getHeader('Authorization');
8483
if (!str_starts_with(strtolower($authHeader), 'basic ')) {
@@ -101,18 +100,8 @@ public function beforeController(Controller $controller, string $methodName) {
101100
}
102101
}
103102

104-
private function needsPasswordConfirmation(ReflectionMethod $reflectionMethod): bool {
105-
$attributes = $reflectionMethod->getAttributes(PasswordConfirmationRequired::class);
106-
if (!empty($attributes)) {
107-
return true;
108-
}
109-
110-
if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) {
111-
$this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . 'PasswordConfirmationRequired' . ' annotation and should use the #[PasswordConfirmationRequired] attribute instead');
112-
return true;
113-
}
114-
115-
return false;
103+
private function needsPasswordConfirmation(): bool {
104+
return $this->reflector->hasAnnotationOrAttribute('PasswordConfirmationRequired', PasswordConfirmationRequired::class);
116105
}
117106

118107
private function isPasswordConfirmationStrict(ReflectionMethod $reflectionMethod): bool {

‎lib/private/AppFramework/Middleware/SessionMiddleware.php‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use OCP\AppFramework\Http\Response;
1515
use OCP\AppFramework\Middleware;
1616
use OCP\ISession;
17-
use ReflectionMethod;
1817

1918
class SessionMiddleware extends Middleware {
2019
public function __construct(
@@ -28,18 +27,7 @@ public function __construct(
2827
* @param string $methodName
2928
*/
3029
public function beforeController($controller, $methodName) {
31-
/**
32-
* Annotation deprecated with Nextcloud 26
33-
*/
34-
$hasAnnotation = $this->reflector->hasAnnotation('UseSession');
35-
if ($hasAnnotation) {
36-
$this->session->reopen();
37-
return;
38-
}
39-
40-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
41-
$hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class));
42-
if ($hasAttribute) {
30+
if ($this->reflector->hasAnnotationOrAttribute('UseSession', UseSession::class)) {
4331
$this->session->reopen();
4432
}
4533
}
@@ -51,18 +39,7 @@ public function beforeController($controller, $methodName) {
5139
* @return Response
5240
*/
5341
public function afterController($controller, $methodName, Response $response) {
54-
/**
55-
* Annotation deprecated with Nextcloud 26
56-
*/
57-
$hasAnnotation = $this->reflector->hasAnnotation('UseSession');
58-
if ($hasAnnotation) {
59-
$this->session->close();
60-
return $response;
61-
}
62-
63-
$reflectionMethod = new ReflectionMethod($controller, $methodName);
64-
$hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class));
65-
if ($hasAttribute) {
42+
if ($this->reflector->hasAnnotationOrAttribute('UseSession', UseSession::class)) {
6643
$this->session->close();
6744
}
6845

0 commit comments

Comments
 (0)