Mitgliederverwaltung/.plans/open/issue-207-authorization-middleware-instanceof-chains.md

Issue #207: Authorization Middleware Uses instanceof Chains

Problem

The AuthorizationMiddleware::getAdminMethods() method uses a chain of instanceof checks to determine admin-only methods:

private function getAdminMethods($controller): ?array {
    if ($controller instanceof FeeController) {
        return self::ADMIN_METHODS_FEE;
    }
    if ($controller instanceof StufeController) {
        return self::ADMIN_METHODS_STUFE;
    }
    if ($controller instanceof ExportController) {
        return self::ADMIN_METHODS_EXPORT;
    }
    if ($controller instanceof MemberController) {
        return self::ADMIN_METHODS_MEMBER;
    }
    // ... more controllers
    return null;
}

This pattern violates Open/Closed Principle — adding a new controller requires modifying the middleware.

Impact

• Hard to maintain as controller count grows
• Easy to forget to add a new controller to the chain
• Mixing of concerns (controller identity vs. permissions)

Solution

Use PHP attributes to declare permissions on controller methods:

#[Attribute]
class RequiresAdmin implements \Attribute {}

#[RequiresAdmin]
public function createRule(): JSONResponse { ... }

Then scan for attributes in middleware:

public function beforeController($controller, $methodName): void {
    $method = new \ReflectionMethod($controller, $methodName);
    if ($method->getAttributes(RequiresAdmin::class)) {
        if (!$this->permissionService->isAdmin($userId)) {
            throw new \RuntimeException('Authorization: admin required');
        }
    }
    // ... rest of checks
}

Tasks

• Create RequiresRead and RequiresWrite attributes
• Create RequiresAdmin attribute
• Add attributes to all controller methods
• Refactor AuthorizationMiddleware to use reflection + attributes
• Remove ADMIN_METHODS_* constants and getAdminMethods() method
• Test all permission levels still work correctly

Labels

• refactoring
• backend
• priority:medium
• architecture