Generate a detailed and accurate OpenAPI spec from the backend

.gitattributes

@@ -0,0 +1,8 @@
+# Generated OpenAPI spec snapshots. They are produced by running the PHPUnit
+# suite with UPDATE_OPENAPI_FIXTURES=1 and are never hand-edited, so collapse
+# them in code review and keep them out of the language statistics.
+ci/phpunit/fixtures/openapi/*.spec.json linguist-generated=true
+
+# The full spec at the repository root is produced by
+# `php ci/tools/generate-openapi.php --pretty` and is never hand-edited either.
+openapi.json linguist-generated=true

.github/openapi/spectral-hashtopolis.yml

@@ -0,0 +1,188 @@
+description: >
+  Lint ruleset for the generated Hashtopolis APIv2 OpenAPI spec.
+
+  It extends the upstream JSON:API styleguide (spectral-jsonapi.yml, kept
+  unmodified so it can be updated from upstream). No rule is switched off: the
+  entries below re-scope rules that upstream states more broadly than JSON:API
+  itself does, because the upstream ruleset is written against JSON:API 1.0
+  while the APIv2 speaks 1.1 (extensions carry the "ext" media type parameter)
+  and answers errors as RFC 7807 problem documents. Every entry names the code
+  that decides the behaviour, so the rule can be restored once the behaviour
+  changes.
+
+  Note that post-409-response and patch-409-response of the upstream ruleset
+  contradict its own post-409-response-code and patch-409-response-code: the
+  first pair requires that no 409 response is documented, the second that one
+  is. They are reported at severity "info", so they are visible without failing
+  the lint, which runs with the default fail severity of "error".
+
+extends:
+  - ./spectral-jsonapi.yml
+
+rules:
+  # Two deviations are folded into this rule:
+  #   - Helper request bodies are flat maps of form fields read by
+  #     AbstractHelperAPI::processPost, not JSON:API documents, so request bodies
+  #     are only checked under /api/v2/ui/. Responses are checked everywhere,
+  #     errors included: ErrorHandler::errorResponse answers JSON:API error
+  #     documents.
+  #   - Upstream forbids media type parameters, which JSON:API 1.0 did. JSON:API
+  #     1.1 requires the "ext" parameter to name the extensions a document
+  #     applies, so the media type of the atomic operations endpoint
+  #     (AbstractModelAPI::atomicOperations) is allowed as well.
+  content-type:
+    description: "JSON:API payloads MUST use the `application/vnd.api+json` media type, with the
+      `ext` parameter naming the extension when one is applied."
+    documentationUrl: "https://jsonapi.org/format/1.1/#extension-negotiation"
+    message: "content MUST be 'application/vnd.api+json', optionally with the 'ext' parameter of an applied extension"
+    severity: error
+    given:
+      - "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))]..requestBody.content"
+      - "$.paths..responses.*.content"
+    then:
+      field: "@key"
+      function: enumeration
+      functionOptions:
+        values:
+          - application/vnd.api+json
+          - application/vnd.api+json;ext="https://jsonapi.org/ext/atomic"
+
+  # The rules below only hold for the JSON:API resource routes under
+  # /api/v2/ui/. The routes under /api/v2/helper/ are RPC style actions and file
+  # transfers: they take a flat body, answer 200 with a meta document
+  # (AbstractBaseAPI::getMetaResponse) and have no conflict semantics.
+  post-2xx-response-codes:
+    description: "`POST` requests MUST support one of the following 2xx codes: 201, 202 or 204."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-creating-responses"
+    message: "POST requests MUST support one Of the following 2xx codes: 201, 202 or 204"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))][post].responses"
+    then:
+      function: schema
+      functionOptions:
+        dialect: "draft2020-12"
+        schema:
+          type: object
+          anyOf:
+            - required: ["201"]
+            - required: ["202"]
+            - required: ["204"]
+
+  post-409-response-code:
+    description: "`POST` requests MUST document and support response code 409."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-creating-responses"
+    message: "POST paths must support response codes: 409"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))][post].responses"
+    then:
+      field: "409"
+      function: truthy
+
+  patch-409-response-code:
+    description: "`PATCH` requests MUST document and support response code 409."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-updating-responses"
+    message: "PATCH requests MUST support response codes: 409"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))][patch].responses"
+    then:
+      field: "409"
+      function: truthy
+
+  patch-404-response-code:
+    description: "`PATCH` requests MUST support response code 404."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-updating-responses"
+    message: "PATCH requests MUST support response code 404"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))][patch].responses"
+    then:
+      field: "404"
+      function: truthy
+
+  delete-404-response-code:
+    description: "`DELETE` requests MUST support response code 404."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-deleting-responses"
+    message: "DELETE requests MUST support response code 404"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))][delete].responses"
+    then:
+      field: "404"
+      function: truthy
+
+  # Upstream applies this to every PATCH body, but a PATCH of a to-many
+  # relationship link MUST carry an array of resource identifiers, so the rule
+  # only holds for the resource routes, exactly like post-requests-single-object
+  # below. Several objects are updated in one request through the atomic
+  # operations endpoint (AbstractModelAPI::atomicOperations), whose body is an
+  # operations document and not a resource object.
+  patch-requests-single-object:
+    description: "A `PATCH` MUST carry a single resource object. Relationship routes are excluded:
+      patching a to-many relationship carries an array of resource identifier objects."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-updating"
+    message: "PATCH requests MAY only contain a single resource object"
+    severity: error
+    given: "$.paths[?(!@property.match(/\\/relationships\\//))].patch.requestBody.content[application/vnd.api+json].schema.properties.data[?(@property==='type' && @ === 'array')]"
+    then:
+      function: falsy
+
+  # Upstream applies this to every POST body, but a POST to a to-many
+  # relationship URL MUST carry an array of resource identifiers, so the rule
+  # only holds for the resource creation routes.
+  post-requests-single-object:
+    description: "A `POST` that creates a resource MUST carry a single resource object.
+      Relationship routes are excluded: posting to a to-many relationship carries an array
+      of resource identifier objects."
+    documentationUrl: "https://jsonapi.org/format/1.0/#crud-creating"
+    message: "POST requests MAY only contain a single resource object"
+    severity: error
+    given: "$.paths[?(!@property.match(/\\/relationships\\//))].post.requestBody.content[application/vnd.api+json].schema.properties.data[?(@property==='type' && @ === 'array')]"
+    then:
+      function: falsy
+
+  # Helper request bodies are flat maps of form fields and helper responses are
+  # meta documents, so the top level document rules only apply under
+  # /api/v2/ui/.
+  top-level-json-properties:
+    description: "The root of a JSON:API document MUST follow the JSON:API document schema."
+    documentationUrl: "https://jsonapi.org/format/1.0/#document-top-level"
+    message: "Root JSON object MUST follow the jsonapi schema"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))]..content['application/vnd.api+json'].schema"
+    then:
+      field: "properties"
+      function: schema
+      functionOptions:
+        dialect: "draft2020-12"
+        schema:
+          type: object
+          anyOf:
+            - required: ["data"]
+            - required: ["errors"]
+            - required: ["meta"]
+          not:
+            anyOf:
+              - required: ["data", "errors"]
+          dependentRequired:
+            included: ["data"]
+
+  top-level-json-object:
+    description: "A JSON:API request or response body MUST be a JSON object."
+    documentationUrl: "https://jsonapi.org/format/1.0/#document-top-level"
+    message: "Request/response body must be wrapped in root level JSON object"
+    severity: error
+    given: "$.paths[?(@property.match(/^\\/api\\/v2\\/ui\\//))]..content['application/vnd.api+json'].schema"
+    then:
+      field: type
+      function: enumeration
+      functionOptions:
+        values:
+          - object
+
+overrides:
+  # /api/v2/auth/token exchanges basic auth credentials for a JWT. It is not a
+  # JSON:API resource endpoint: it takes a scope list and answers a plain
+  # application/json body (see token.routes.php), which is the one media type
+  # the content-type rule cannot be satisfied with. The other rules hold for it.
+  - files:
+      - "**#/paths/~1api~1v2~1auth~1token"
+    rules:
+      content-type: off

.github/workflows/openapi-lint.yml

@@ -37,4 +37,4 @@ jobs:
       - name: Lint OpenAPI schema with Redocly
         run: redocly lint openapi.json
       - name: Lint OpenAPI schema with Spectral
-        run: spectral lint openapi.json --ruleset .github/openapi/spectral-jsonapi.yml -D
+        run: spectral lint openapi.json --ruleset .github/openapi/spectral-hashtopolis.yml -D

.phpactor.json

@@ -0,0 +1,6 @@
+{
+    "$schema": "/phpactor.schema.json",
+    "language_server_phpstan.enabled": false,
+    "php_code_sniffer.enabled": false,
+    "prophecy.enabled": false
+}

ci/apiv2/test_agent.py

@@ -3,7 +3,7 @@
 from hashtopolis import Agent, Config, Helper
 from hashtopolis import HashtopolisError
 
-from utils import BaseTest
+from utils import BaseTest, error_title, patch_many
 
 
 class AgentTest(BaseTest):
@@ -54,7 +54,7 @@ def test_name_too_long(self):
         with self.assertRaises(HashtopolisError) as e:
             self._test_patch(model_obj, 'agentName', too_long_name)  # name exceeds max size of 100
         self.assertEqual(e.exception.status_code, 400)
-        self.assertEqual(e.exception.title, f"The string value: '{too_long_name}' is too long. The max size is '100'")
+        self.assertEqual(error_title(e.exception), f"The string value: '{too_long_name}' is too long. The max size is '100'")
 
     def test_expandables(self):
         model_obj = self.create_test_object()
@@ -83,7 +83,7 @@ def test_assign_unassign_agent(self):
     def test_bulk_activate(self):
         agents = [self.create_agent() for i in range(5)]
         active_attributes = [True for i in range(5)]
-        Agent.objects.patch_many(agents, active_attributes, "isActive")
+        patch_many(Agent, agents, active_attributes, "isActive")
 
     def test_hide_ip_info(self):
         agent_obj = self.create_test_object()

ci/apiv2/test_atomic_operations.py

@@ -0,0 +1,103 @@
+import datetime
+import json
+
+import requests
+
+from hashtopolis import AccessGroup, HashtopolisConfig, HashtopolisConnector
+from utils import BaseTest
+
+# JSON:API 1.1 requires the extension to be named in the media type of both the
+# request and the response (https://jsonapi.org/ext/atomic/).
+ATOMIC_MEDIA_TYPE = 'application/vnd.api+json;ext="https://jsonapi.org/ext/atomic"'
+
+
+class AtomicOperationsTest(BaseTest):
+    """The JSON:API atomic operations endpoint of a collection, POST .../operations."""
+
+    model_class = AccessGroup
+
+    def create_test_object(self, *nargs, **kwargs):
+        return self.create_accessgroup(*nargs, **kwargs)
+
+    def _unique_name(self, suffix=''):
+        return f'Testing Group {datetime.datetime.now().isoformat()}{suffix}'
+
+    def _post(self, operations, content_type=ATOMIC_MEDIA_TYPE):
+        conn = HashtopolisConnector('/ui/accessgroups', HashtopolisConfig())
+        conn.authenticate()
+        headers = dict(conn._headers)
+        headers['Content-Type'] = content_type
+        uri = conn._api_endpoint + conn._model_uri + '/operations'
+        return requests.post(uri, headers=headers, data=json.dumps({'atomic:operations': operations}))
+
+    def test_add_and_update_report_the_written_objects(self):
+        existing = self.create_test_object()
+        added_name = self._unique_name('-added')
+        updated_name = self._unique_name('-updated')
+
+        r = self._post([
+            {'op': 'add', 'data': {'type': 'accessGroup', 'attributes': {'groupName': added_name}}},
+            {'op': 'update', 'data': {'type': 'accessGroup', 'id': str(existing.id),
+                                      'attributes': {'groupName': updated_name}}},
+        ])
+
+        self.assertEqual(r.status_code, 200)
+        self.assertIn('ext="https://jsonapi.org/ext/atomic"', r.headers.get('Content-Type'))
+
+        results = r.json()['atomic:results']
+        self.assertEqual(len(results), 2)
+        self.assertEqual(results[0]['data']['type'], 'accessGroup')
+        self.assertEqual(results[0]['data']['attributes']['groupName'], added_name)
+        self.assertEqual(results[1]['data']['id'], str(existing.id))
+        self.assertEqual(results[1]['data']['attributes']['groupName'], updated_name)
+
+        added = AccessGroup.objects.get(pk=int(results[0]['data']['id']))
+        self.delete_after_test(added)
+        self.assertEqual(added.groupName, added_name)
+        self.assertEqual(AccessGroup.objects.get(pk=existing.id).groupName, updated_name)
+
+    def test_removals_only_are_answered_without_a_body(self):
+        group = self.create_test_object(delete=False)
+
+        r = self._post([{'op': 'remove', 'ref': {'type': 'accessGroup', 'id': str(group.id)}}])
+
+        self.assertEqual(r.status_code, 204)
+        self.assertEqual(r.text, '')
+        self.assertEqual(list(AccessGroup.objects.filter(id=group.id)), [])
+
+    def test_a_failing_operation_undoes_the_preceding_ones(self):
+        groups_before = len(list(AccessGroup.objects.all()))
+
+        r = self._post([
+            {'op': 'add', 'data': {'type': 'accessGroup', 'attributes': {'groupName': self._unique_name()}}},
+            {'op': 'update', 'data': {'type': 'accessGroup', 'id': '99999999',
+                                      'attributes': {'groupName': self._unique_name()}}},
+        ])
+
+        self.assertEqual(r.status_code, 404)
+        # The object created by the first operation must be gone again
+        self.assertEqual(len(list(AccessGroup.objects.all())), groups_before)
+
+    def test_operations_must_address_the_type_of_the_collection(self):
+        r = self._post([{'op': 'remove', 'ref': {'type': 'agent', 'id': '1'}}])
+
+        self.assertEqual(r.status_code, 400)
+        self.assertIn('accessGroup', r.json()['errors'][0]['title'])
+
+    def test_unknown_operations_are_rejected(self):
+        r = self._post([{'op': 'replace', 'data': {'type': 'accessGroup', 'attributes': {}}}])
+
+        self.assertEqual(r.status_code, 400)
+
+    def test_the_extension_must_be_named_in_the_content_type(self):
+        for content_type in ['application/vnd.api+json', 'application/json']:
+            r = self._post([{'op': 'remove', 'ref': {'type': 'accessGroup', 'id': '1'}}], content_type=content_type)
+            self.assertEqual(r.status_code, 415, f'Content-Type: {content_type}')
+
+    def test_an_unusable_media_type_parameter_is_rejected(self):
+        """Content negotiation of JSON:API 1.1: only ext and profile may modify the media type."""
+        r = self._post([], content_type='application/vnd.api+json;charset=utf-8')
+        self.assertEqual(r.status_code, 415)
+
+        r = self._post([], content_type='application/vnd.api+json;ext="https://example.com/ext/unknown"')
+        self.assertEqual(r.status_code, 415)

ci/apiv2/test_attributes.py

@@ -4,7 +4,7 @@
 
 from hashtopolis import HashtopolisConnector, HashtopolisConfig, HashtopolisError
 from hashtopolis import User
-from utils import BaseTest
+from utils import BaseTest, error_title
 
 
 class AttributeTypeTest(BaseTest):
@@ -34,7 +34,7 @@ def test_patch_read_only(self):
         r = requests.patch(uri, headers=headers, data=json.dumps(payload))
 
         self.assertEqual(r.status_code, 403)
-        self.assertIn('immutable', r.json().get('title'))
+        self.assertIn('immutable', r.json()['errors'][0]['title'])
         user.delete()
 
     def test_create_protected(self):
@@ -52,7 +52,7 @@ def test_create_protected(self):
             user.save()
 
         self.assertEqual(e.exception.status_code, 403)
-        self.assertIn(' not valid input ', e.exception.title)
+        self.assertIn(' not valid input ', error_title(e.exception))
 
     def test_get_private(self):
         stamp = int(time.time() * 1000)