Enforce admin rquirements in NCNv1

CHANGELOG

@@ -13,6 +13,10 @@ Bug fixes:
   compatibility with RFC 7617
 - Accept "t" and "f" as booleans in Tiny Tiny RSS
 
+Changes:
+- Administrator account requirements for Nextcloud News functionality are
+  now enforced
+
 Version 0.8.5 (2020-10-27)
 ==========================
 

docs/en/030_Supported_Protocols/010_Nextcloud_News.md

@@ -24,7 +24,6 @@ It allows organizing newsfeeds into single-level folders, and supports a wide ra
 - When marking articles as starred the feed ID is ignored, as they are not needed to establish uniqueness
 - The feed updater ignores the `userId` parameter: feeds in The Arsse are deduplicated, and have no owner
 - The `/feeds/all` route lists only feeds which should be checked for updates, and it also returns all `userId` attributes as empty strings: feeds in The Arsse are deduplicated, and have no owner
-- The API's "updater" routes do not require administrator priviledges as The Arsse has no concept of user classes
 - The "updater" console commands mentioned in the protocol specification are not implemented, as The Arsse does not implement the required Nextcloud subsystems
 - The `lastLoginTimestamp` attribute of the user metadata is always the current time: The Arsse's implementation of the protocol is fully stateless
 - Syntactically invalid JSON input will yield a `400 Bad Request` response instead of falling back to GET parameters

lib/REST/NextcloudNews/V1_2.php

@@ -360,6 +360,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
 
     // return list of feeds which should be refreshed
     protected function feedListStale(array $url, array $data): ResponseInterface {
+        if (!$this->isAdmin()) {
+            return new EmptyResponse(403);
+        }
         // list stale feeds which should be checked for updates
         $feeds = Arsse::$db->feedListStale();
         $out = [];
@@ -372,6 +375,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
 
     // refresh a feed
     protected function feedUpdate(array $url, array $data): ResponseInterface {
+        if (!$this->isAdmin()) {
+            return new EmptyResponse(403);
+        }
         try {
             Arsse::$db->feedUpdate($data['feedId']);
         } catch (ExceptionInput $e) {
@@ -667,11 +673,17 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
     }
 
     protected function cleanupBefore(array $url, array $data): ResponseInterface {
+        if (!$this->isAdmin()) {
+            return new EmptyResponse(403);
+        }
         Service::cleanupPre();
         return new EmptyResponse(204);
     }
 
     protected function cleanupAfter(array $url, array $data): ResponseInterface {
+        if (!$this->isAdmin()) {
+            return new EmptyResponse(403);
+        }
         Service::cleanupPost();
         return new EmptyResponse(204);
     }

tests/cases/REST/NextcloudNews/TestV1_2.php

@@ -317,6 +317,7 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         // create a mock user manager
         Arsse::$user = \Phake::mock(User::class);
         Arsse::$user->id = "john.doe@example.com";
+        \Phake::when(Arsse::$user)->propertiesGet->thenReturn(['admin' => true]);
         // create a mock database interface
         Arsse::$db = \Phake::mock(Database::class);
         $this->transaction = \Phake::mock(Transaction::class);
@@ -629,6 +630,13 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         $this->assertMessage($exp, $this->req("GET", "/feeds/all"));
     }
 
+    public function testListStaleFeedsWithoutAuthority(): void {
+        \Phake::when(Arsse::$user)->propertiesGet->thenReturn(['admin' => false]);
+        $exp = new EmptyResponse(403);
+        $this->assertMessage($exp, $this->req("GET", "/feeds/all"));
+        \Phake::verify(Arsse::$db, \Phake::times(0))->feedListStale;
+    }
+
     public function testUpdateAFeed(): void {
         $in = [
             ['feedId' =>    42], // valid
@@ -650,6 +658,13 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         $this->assertMessage($exp, $this->req("GET", "/feeds/update", json_encode($in[4])));
     }
 
+    public function testUpdateAFeedWithoutAuthority(): void {
+        \Phake::when(Arsse::$user)->propertiesGet->thenReturn(['admin' => false]);
+        $exp = new EmptyResponse(403);
+        $this->assertMessage($exp, $this->req("GET", "/feeds/update", ['feedId' => 42]));
+        \Phake::verify(Arsse::$db, \Phake::times(0))->feedUpdate;
+    }
+
     /** @dataProvider provideArticleQueries */
     public function testListArticles(string $url, array $in, Context $c, $out, ResponseInterface $exp): void {
         if ($out instanceof \Exception) {
@@ -849,6 +864,13 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         \Phake::verify(Arsse::$db)->feedCleanup();
     }
 
+    public function testCleanUpBeforeUpdateWithoutAuthority(): void {
+        \Phake::when(Arsse::$user)->propertiesGet->thenReturn(['admin' => false]);
+        $exp = new EmptyResponse(403);
+        $this->assertMessage($exp, $this->req("GET", "/cleanup/before-update"));
+        \Phake::verify(Arsse::$db, \Phake::times(0))->feedCleanup;
+    }
+
     public function testCleanUpAfterUpdate(): void {
         \Phake::when(Arsse::$db)->articleCleanup()->thenReturn(true);
         $exp = new EmptyResponse(204);
@@ -856,6 +878,13 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         \Phake::verify(Arsse::$db)->articleCleanup();
     }
 
+    public function testCleanUpAfterUpdateWithoutAuthority(): void {
+        \Phake::when(Arsse::$user)->propertiesGet->thenReturn(['admin' => false]);
+        $exp = new EmptyResponse(403);
+        $this->assertMessage($exp, $this->req("GET", "/cleanup/after-update"));
+        \Phake::verify(Arsse::$db, \Phake::times(0))->feedCleanup;
+    }
+
     public function testQueryTheUserStatus(): void {
         $act = $this->req("GET", "/user");
         $exp = new Response([