feat(adr-005): per-object auth demo on DELETE /api/items/{id}

Demonstrates the ADR-005 mandate that #[NoAdminRequired] mutation
endpoints MUST carry an explicit backend auth check. The template had
no domain mutation endpoint to demo this on, so the audit listed it as
'partial' — this commit adds a minimal, copy-pasteable example.

- lib/Service/ItemService::delete($id, $userId) enforces an admin-OR-owner
  check via IGroupManager::isAdmin() and the OpenRegister @self.owner
  metadata, returning a STATUS_* enum the controller maps to HTTP codes.
- lib/Controller/ItemController::destroy($id) declares #[NoAdminRequired],
  derives the acting UID from IUserSession (never a request param), and
  maps STATUS_DELETED→204, STATUS_FORBIDDEN→403, STATUS_NOT_FOUND→404,
  STATUS_UNAVAILABLE→503, Throwable→generic 500 with server-side log.
- appinfo/routes.php registers DELETE /api/items/{id} before the SPA
  catch-all, per existing route-ordering comments.
- Follows OWASP A01: returns 404 (not 403) for non-existent IDs so
  response codes don't leak existence.
- ItemServiceTest (6 methods) and ItemControllerTest (5 methods) lock
  down all auth branches + the HTTP mapping. Full unit suite now runs
  26 tests / 78 assertions, all passing.

Author: rubenvdlinde

appinfo/routes.php

@@ -25,6 +25,10 @@
         ['name' => 'settings#create', 'url' => '/api/settings', 'verb' => 'POST'],
         ['name' => 'settings#load', 'url' => '/api/settings/load', 'verb' => 'POST'],
 
+        // Items API — demonstrates ADR-005 #[NoAdminRequired] + per-object auth
+        // on a mutation (admin OR owner check lives in ItemService::delete()).
+        ['name' => 'item#destroy', 'url' => '/api/items/{id}', 'verb' => 'DELETE'],
+
         // Prometheus metrics endpoint (ADR-006) — admin auth.
         ['name' => 'metrics#index', 'url' => '/api/metrics', 'verb' => 'GET'],
 

lib/Controller/ItemController.php

@@ -0,0 +1,117 @@
+<?php
+
+/**
+ * AppTemplate Item Controller
+ *
+ * Controller for Article (item) mutation endpoints. Demonstrates the ADR-005
+ * `#[NoAdminRequired]` + per-object auth pattern on DELETE /api/items/{id}.
+ *
+ * @category Controller
+ * @package  OCA\AppTemplate\Controller
+ *
+ * @author    Conduction Development Team <info@conduction.nl>
+ * @copyright 2026 Conduction B.V.
+ * @license   EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
+ *
+ * @version GIT: <git-id>
+ *
+ * @link https://conduction.nl
+ *
+ * @spec openspec/changes/example-change/tasks.md#task-5
+ */
+
+declare(strict_types=1);
+
+namespace OCA\AppTemplate\Controller;
+
+use OCA\AppTemplate\AppInfo\Application;
+use OCA\AppTemplate\Service\ItemService;
+use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\Http\Response;
+use OCP\IRequest;
+use OCP\IUserSession;
+use Psr\Log\LoggerInterface;
+
+/**
+ * Controller for Article (item) mutations.
+ *
+ * Demonstrates ADR-003 (Controller → Service layering: the controller only
+ * maps service results to HTTP codes) and ADR-005 (per-object authorization
+ * on mutations: `#[NoAdminRequired]` paired with an explicit backend check
+ * in the service).
+ */
+class ItemController extends Controller
+{
+    /**
+     * Constructor for ItemController.
+     *
+     * @param IRequest        $request     The HTTP request
+     * @param ItemService     $itemService The item service (owns auth rules)
+     * @param IUserSession    $userSession The user session (backend-derived UID)
+     * @param LoggerInterface $logger      The logger (server-side error logging)
+     *
+     * @return void
+     *
+     * @spec openspec/changes/example-change/tasks.md#task-5
+     */
+    public function __construct(
+        IRequest $request,
+        private readonly ItemService $itemService,
+        private readonly IUserSession $userSession,
+        private readonly LoggerInterface $logger,
+    ) {
+        parent::__construct(appName: Application::APP_ID, request: $request);
+    }//end __construct()
+
+    /**
+     * Delete an Article object by UUID.
+     *
+     * `#[NoAdminRequired]` lets non-admins reach the endpoint; authorization
+     * is enforced inside `ItemService::delete()` via an admin-or-owner check
+     * (ADR-005). The controller ONLY maps the service's STATUS_* result to
+     * the correct HTTP code:
+     *
+     * - 204 No Content — deleted
+     * - 403 Forbidden — authenticated but not authorized for THIS object
+     * - 404 Not Found — object does not exist (or register not configured)
+     * - 503 Service Unavailable — OpenRegister missing
+     *
+     * Error bodies use static generic messages (ADR-005) — the real error is
+     * logged server-side and never leaked to the client.
+     *
+     * @param string $id UUID of the Article object to delete
+     *
+     * @return Response
+     *
+     * @spec openspec/changes/example-change/tasks.md#task-5
+     */
+    #[NoAdminRequired]
+    public function destroy(string $id): Response
+    {
+        $user = $this->userSession->getUser();
+        if ($user === null) {
+            return new JSONResponse(['message' => 'Authentication required'], Http::STATUS_UNAUTHORIZED);
+        }
+
+        try {
+            $status = $this->itemService->delete(id: $id, userId: $user->getUID());
+        } catch (\Throwable $e) {
+            $this->logger->error(
+                'AppTemplate: failed to delete item',
+                ['exception' => $e]
+            );
+            return new JSONResponse(['message' => 'Operation failed'], Http::STATUS_INTERNAL_SERVER_ERROR);
+        }
+
+        return match ($status) {
+            ItemService::STATUS_DELETED     => new Response(Http::STATUS_NO_CONTENT),
+            ItemService::STATUS_FORBIDDEN   => new JSONResponse(['message' => 'Forbidden'], Http::STATUS_FORBIDDEN),
+            ItemService::STATUS_NOT_FOUND   => new JSONResponse(['message' => 'Not found'], Http::STATUS_NOT_FOUND),
+            ItemService::STATUS_UNAVAILABLE => new JSONResponse(['message' => 'Service unavailable'], Http::STATUS_SERVICE_UNAVAILABLE),
+            default                         => new JSONResponse(['message' => 'Operation failed'], Http::STATUS_INTERNAL_SERVER_ERROR),
+        };
+    }//end destroy()
+}//end class

lib/Service/ItemService.php

@@ -0,0 +1,198 @@
+<?php
+
+/**
+ * AppTemplate Item Service
+ *
+ * Service for domain-level operations on Article objects, including the ADR-005
+ * per-object authorization check demonstrated on delete().
+ *
+ * @category Service
+ * @package  OCA\AppTemplate\Service
+ *
+ * @author    Conduction Development Team <info@conduction.nl>
+ * @copyright 2026 Conduction B.V.
+ * @license   EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
+ *
+ * @version GIT: <git-id>
+ *
+ * @link https://conduction.nl
+ *
+ * @spec openspec/changes/example-change/tasks.md#task-5
+ */
+
+declare(strict_types=1);
+
+namespace OCA\AppTemplate\Service;
+
+use OCA\AppTemplate\AppInfo\Application;
+use OCP\App\IAppManager;
+use OCP\IAppConfig;
+use OCP\IGroupManager;
+use Psr\Container\ContainerInterface;
+
+/**
+ * Service for Article (item) operations.
+ *
+ * ## ADR-005 per-object authorization
+ *
+ * `delete()` demonstrates the `#[NoAdminRequired]` + per-object auth pattern
+ * documented in ADR-005 and OWASP A01 (Broken Access Control):
+ *
+ * 1. The controller declares `#[NoAdminRequired]` so non-admins can reach it.
+ * 2. This service performs an **explicit backend check** that the caller is
+ *    either (a) a Nextcloud group admin, or (b) the owner of the object
+ *    (matched via the OpenRegister `@self.owner` metadata).
+ * 3. On failure the service returns a `Status` enum value the controller maps
+ *    to the right HTTP code (403/404/204) — controllers are thin, services
+ *    own the rules.
+ *
+ * The auth check MUST run on the backend, not the frontend, because the
+ * frontend is untrusted (ADR-005). Returning 403 vs 404 also matters for
+ * OWASP A01: a plain 403 on non-existent IDs leaks existence, so we return
+ * 404 for "not found" and 403 only when the object exists but the caller
+ * cannot touch it.
+ */
+class ItemService
+{
+
+    /**
+     * Result status enum used by delete() so the controller can map to HTTP codes.
+     */
+    public const STATUS_DELETED = 'deleted';
+
+    public const STATUS_NOT_FOUND = 'not_found';
+
+    public const STATUS_FORBIDDEN = 'forbidden';
+
+    public const STATUS_UNAVAILABLE = 'unavailable';
+
+    /**
+     * Constructor for ItemService.
+     *
+     * @param IAppConfig         $appConfig    The app config interface
+     * @param IAppManager        $appManager   The app manager (to check OpenRegister presence)
+     * @param IGroupManager      $groupManager The group manager (admin check)
+     * @param ContainerInterface $container    The DI container (lazy-resolves
+     *                                         OpenRegister's ObjectService so
+     *                                         the app still loads when
+     *                                         OpenRegister is absent)
+     *
+     * @return void
+     *
+     * @spec openspec/changes/example-change/tasks.md#task-5
+     */
+    public function __construct(
+        private readonly IAppConfig $appConfig,
+        private readonly IAppManager $appManager,
+        private readonly IGroupManager $groupManager,
+        private readonly ContainerInterface $container,
+    ) {
+    }//end __construct()
+
+    /**
+     * Delete an Article object, enforcing per-object authorization.
+     *
+     * Callers must be either a Nextcloud group admin or the OpenRegister
+     * owner of the object. Returns a STATUS_* constant the controller maps
+     * to the right HTTP response:
+     *
+     * - STATUS_DELETED     → 204 No Content
+     * - STATUS_NOT_FOUND   → 404
+     * - STATUS_FORBIDDEN   → 403
+     * - STATUS_UNAVAILABLE → 503 (OpenRegister missing)
+     *
+     * @param string $id     UUID of the Article object to delete
+     * @param string $userId UID of the acting user (always backend-derived,
+     *                       NEVER taken from a request parameter)
+     *
+     * @return string One of the STATUS_* constants.
+     *
+     * @spec openspec/changes/example-change/tasks.md#task-5
+     */
+    public function delete(string $id, string $userId): string
+    {
+        if ($this->appManager->isInstalled('openregister') === false) {
+            return self::STATUS_UNAVAILABLE;
+        }
+
+        $objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService');
+
+        $register = $this->appConfig->getValueString(Application::APP_ID, 'register', '');
+        if ($register === '') {
+            // Template isn't configured yet — no register to delete from.
+            return self::STATUS_NOT_FOUND;
+        }
+
+        $object = $objectService->find(register: $register, schema: 'article', id: $id);
+        if ($object === null) {
+            // OWASP A01: return 404 for non-existent objects — do NOT 403, which
+            // would leak existence. Only returned once we know the object is not there.
+            return self::STATUS_NOT_FOUND;
+        }
+
+        if ($this->isAuthorized(object: $object, userId: $userId) === false) {
+            return self::STATUS_FORBIDDEN;
+        }
+
+        $objectService->delete(register: $register, schema: 'article', id: $id);
+
+        return self::STATUS_DELETED;
+    }//end delete()
+
+    /**
+     * Check whether the user is allowed to mutate the given object.
+     *
+     * Authorization rule: caller is a Nextcloud group admin OR is the
+     * OpenRegister owner of the object (via `@self.owner`).
+     *
+     * @param object|array<string,mixed> $object The object as returned by OpenRegister
+     *                                          (may be an entity or an associative array)
+     * @param string                     $userId The acting user's UID
+     *
+     * @return bool True if authorized, false otherwise.
+     *
+     * @spec openspec/changes/example-change/tasks.md#task-5
+     */
+    private function isAuthorized(object|array $object, string $userId): bool
+    {
+        if ($this->groupManager->isAdmin($userId) === true) {
+            return true;
+        }
+
+        $owner = $this->extractOwner($object);
+        return ($owner !== null && $owner === $userId);
+    }//end isAuthorized()
+
+    /**
+     * Extract the owner UID from an OpenRegister object, tolerating both
+     * the entity shape (getOwner() / jsonSerialize()['@self']['owner'])
+     * and the plain associative-array shape.
+     *
+     * @param object|array<string,mixed> $object The object to inspect
+     *
+     * @return string|null The owner UID, or null if it cannot be determined.
+     *
+     * @spec openspec/changes/example-change/tasks.md#task-5
+     */
+    private function extractOwner(object|array $object): ?string
+    {
+        if (is_array($object) === true) {
+            $self = ($object['@self'] ?? []);
+            return is_array($self) === true ? (($self['owner'] ?? null)) : null;
+        }
+
+        if (method_exists($object, 'getOwner') === true) {
+            $owner = $object->getOwner();
+            return is_string($owner) === true ? $owner : null;
+        }
+
+        if (method_exists($object, 'jsonSerialize') === true) {
+            $serialised = $object->jsonSerialize();
+            if (is_array($serialised) === true && isset($serialised['@self']['owner']) === true) {
+                return (string) $serialised['@self']['owner'];
+            }
+        }
+
+        return null;
+    }//end extractOwner()
+}//end class