From 55f76570db4c6469df0a4104d5d0b97fe570e04d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 23 Apr 2024 19:44:30 +0100 Subject: [PATCH 01/10] Improve comments in `render-object-table` Just documenting the current situation. --- .../partials/openapi/render-object-table.html | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/layouts/partials/openapi/render-object-table.html b/layouts/partials/openapi/render-object-table.html index 6faa21d9f..2c59401d2 100644 --- a/layouts/partials/openapi/render-object-table.html +++ b/layouts/partials/openapi/render-object-table.html @@ -9,11 +9,11 @@ * `properties`: optional dictionary of the properties to list, each given as: `property_name` : `property_data` - * `additionalProperties`: optional dictionary for properties with undefined - names, in the same format as `property_data` + * `additionalProperties`: an OpenAPI schema document for additional properties + on the object. * `patternProperties`: optional dictionary for properties with names adhering - to a regex pattern, in the same format as `property_data` + to a regex pattern. A map from regex pattern to OpenAPI schema document. * `required`: optional array containing the names of required properties. In some cases (such as response body specifications) this isn't used, and @@ -55,6 +55,15 @@ {{ else if (or .additionalProperties .patternProperties) }} + +{{/* +A special format of table for objects which only have additionalProperties or patternProperties. + +This is only ever used for top-level objects. Nested objects in this situation are just shown +as rows within their parent object, and don't get their own table. (They are filtered out in +resolve-additional-types.) +*/}} + {{ with $title }} {{ . }} @@ -113,7 +122,7 @@ {{ else if or (reflect.IsSlice .type) .oneOf }} {{/* It's legal to specify an array of types. - + There are two ways to do that: - Use an array of strings. - Use oneOf, with items having a schema. From 6f403f53b53061dcd3004999a3afcd402b34eeb5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 23 Apr 2024 19:09:40 +0100 Subject: [PATCH 02/10] Pass `additionalProperties` and `patternProperties` into `render-object-table` Previously, we were stripping `additionalProperties` and `patternProperties` from all objects except top-level objects. Obviously, this was no good for objects where a nested property contains such properties. Fixing that (in `clean-object`) *ought* to be simple enough, except that it turned out we were relying on the fact that would give us an "empty" entry in the array of types-to-render-tables-for returned by `resolve-additional-types`. (Normally, we don't want an object that only has `additionalProperties` to have its own table, since we just embed it in the parent table.) So, we need to add more logic to `resolve-additional-types-inner` to suppress such tables. This commit doesn't change the rendered output at all (verified via `diff`): it's just preparation for what comes next. --- .../json-schema/resolve-additional-types.html | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/layouts/partials/json-schema/resolve-additional-types.html b/layouts/partials/json-schema/resolve-additional-types.html index 157045b56..f210d0f78 100644 --- a/layouts/partials/json-schema/resolve-additional-types.html +++ b/layouts/partials/json-schema/resolve-additional-types.html @@ -29,6 +29,8 @@ * * * title * * properties + * * additionalProperties + * * patternProperties * * required * * enum * * anchor @@ -45,27 +47,31 @@ "schema" .schema "anchor_base" .anchor_base "name" (.name | default .schema.title | default "") + "top_level" true ) }} {{ return $res.objects }} /* * A helper for the resolve-additional-types partial. * - * Takes the same inputs as resolve-additional-types itself, except that `name` is mandatory. + * Takes the same inputs as resolve-additional-types itself, except: + * * `name` is mandatory. + * * `top_level`, if set, indicates that this is the top-level object. * * Returns a dict containing: * * * `objects`: The array of object schema definitions. * - * * `schema`: An updated copy of the `schema` input (eg, nested `allOf` - * entries are expanded, and an `anchor` may be added). If the input - * `schema` was itself an object, this will be the same as the first entry - * in `objects`. + * * `schema`: An updated copy of the `schema` input (eg, an `anchor` may be added). + * If the input `schema` was itself an object that we should create a table for, + * this will be the same as the first entry in `objects`. */ {{ define "partials/resolve-additional-types-inner" }} {{ $this_object := .schema }} {{ $anchor_base := .anchor_base }} {{ $name := .name }} + {{ $top_level := .top_level }} + {{ $all_objects := slice }} {{ if eq $this_object.type "object" }} @@ -126,12 +132,24 @@ {{ $this_object = merge $this_object (dict "properties" $updated_properties) }} {{ end }} - /* Finally, prepend the updated schema for the top-level object onto the $all_objects array */ - {{ $tmp := slice $this_object }} - {{ if $all_objects }} - {{ $tmp = $tmp | append $all_objects }} + /* We'll want to create a table for `$this_object` itself if either: + * + * - The object has some regular properties (not just patternProperties or additionalProperties), or: + * - It is the top-level object. (We use a special format of table for top-level objects that + * only have patternProperties or additionalProperties) + * + * In those cases, prepend the updated schema for the top-level object onto the $all_objects array. + * + * We think about this last so that we can take advantage of any updates to the schema that happened + * above (in particular, addition of `anchor` attributes). + */ + {{ if or $this_object.properties $top_level }} + {{ $tmp := slice $this_object }} + {{ if $all_objects }} + {{ $tmp = $tmp | append $all_objects }} + {{ end }} + {{ $all_objects = $tmp }} {{ end }} - {{ $all_objects = $tmp }} {{ end }} {{ if eq $this_object.type "array" }} @@ -193,8 +211,7 @@ * * Returns a dict containing: * * `objects`: The array of object schema definitions. - * * `schema`: An updated copy of the `schema` input (eg, nested `allOf` - * entries are expanded, and an `anchor` may be added). + * * `schema`: An updated copy of the `schema` input (eg, an `anchor` may be added). */ {{ define "partials/get-additional-objects" }} /* .name is the name of the object for logging purposes */ @@ -206,7 +223,7 @@ {{ errorf "Invalid call to partials/get-additional-objects: %s is not a map" $name .this_object }} {{ end }} - {{ $res := partial "resolve-additional-types-inner" (dict "schema" .this_object "anchor_base" .anchor_base "name" $name) }} + {{ $res := partial "resolve-additional-types-inner" (dict "schema" .this_object "anchor_base" .anchor_base "name" $name "top_level" false) }} {{ range $res.objects }} {{ $all_objects = $all_objects | append (partial "clean-object" .) }} {{ end }} @@ -221,5 +238,5 @@ * but with (for example) different examples will be considered different. */ {{ define "partials/clean-object" }} - {{ return (dict "title" .title "properties" .properties "required" .required "enum" .enum "anchor" .anchor) }} + {{ return (dict "title" .title "properties" .properties "additionalProperties" .additionalProperties "patternProperties" .patternProperties "required" .required "enum" .enum "anchor" .anchor) }} {{ end }} From 6aabf113b8698769dc9fcb83fa0d9ab913ea40f9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 23 Apr 2024 19:17:22 +0100 Subject: [PATCH 03/10] Include `additionalProperties` data in object tables --- .../partials/openapi/render-object-table.html | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/layouts/partials/openapi/render-object-table.html b/layouts/partials/openapi/render-object-table.html index 2c59401d2..b8903a4e6 100644 --- a/layouts/partials/openapi/render-object-table.html +++ b/layouts/partials/openapi/render-object-table.html @@ -26,7 +26,6 @@ {{ $required := .required}} {{ if $properties }} - {{ with $title }} {{ . }} @@ -52,6 +51,28 @@ {{ end }} + {{/* + If the object has additional properties *as well as* regular properties, we add a special row to the table. + + Note that, per https://json-schema.org/draft/2020-12/json-schema-core#name-boolean-json-schemas, JSON schemas + can be a simple "true" or "false" as well as the more normal object. + + `additionalProperties: true` is pretty much the default for Matrix (it means: "you're allowed to include random + unspecced properties in your object"), so nothing to do here. + + `additionalProperties: false` means "you're not allowed to include any unspecced properties in your object". We + may want to consider how to display that; for now we just ignore it. + + TODO: support `patternProperties` here. + */}} + {{ if reflect.IsMap .additionalProperties }} + + + <Other properties> + {{ partial "partials/property-type" .additionalProperties }} + {{ partial "partials/property-description" (dict "property" .additionalProperties) }} + + {{ end }} {{ else if (or .additionalProperties .patternProperties) }} From b020b1d5c01b4b72a79b9978844025e5932bf8e0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 23 Apr 2024 19:21:04 +0100 Subject: [PATCH 04/10] changelog --- changelogs/internal/newsfragments/1798.clarification | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/internal/newsfragments/1798.clarification diff --git a/changelogs/internal/newsfragments/1798.clarification b/changelogs/internal/newsfragments/1798.clarification new file mode 100644 index 000000000..5bd28a93c --- /dev/null +++ b/changelogs/internal/newsfragments/1798.clarification @@ -0,0 +1 @@ +Show information about "Additional Properties" in object tables. From f2ebb4003b4f0b9048c453510a85dfd1107bbf11 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 24 Apr 2024 11:07:06 +0100 Subject: [PATCH 05/10] Address review comments --- data/api/identity/v2_terms.yaml | 1 - layouts/partials/openapi/render-object-table.html | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/data/api/identity/v2_terms.yaml b/data/api/identity/v2_terms.yaml index d9eea90e2..d7c231d4b 100644 --- a/data/api/identity/v2_terms.yaml +++ b/data/api/identity/v2_terms.yaml @@ -52,7 +52,6 @@ paths: might be and could be "alpha", semantically versioned, or arbitrary. required: - version - # TODO: TravisR - Make this render additionalProperties: type: object title: Internationalised Policy diff --git a/layouts/partials/openapi/render-object-table.html b/layouts/partials/openapi/render-object-table.html index b8903a4e6..71bca884a 100644 --- a/layouts/partials/openapi/render-object-table.html +++ b/layouts/partials/openapi/render-object-table.html @@ -9,11 +9,11 @@ * `properties`: optional dictionary of the properties to list, each given as: `property_name` : `property_data` - * `additionalProperties`: an OpenAPI schema document for additional properties - on the object. + * `additionalProperties`: a JSON Schema for additional properties on the + object. * `patternProperties`: optional dictionary for properties with names adhering - to a regex pattern. A map from regex pattern to OpenAPI schema document. + to a regex pattern. A map from regex pattern to JSON Schema. * `required`: optional array containing the names of required properties. In some cases (such as response body specifications) this isn't used, and @@ -69,7 +69,7 @@ <Other properties> - {{ partial "partials/property-type" .additionalProperties }} + {{ partial "partials/property-type" .additionalProperties | safeHTML }} {{ partial "partials/property-description" (dict "property" .additionalProperties) }} {{ end }} From c0dc86338486d50e99fb9eb89b720bd02fcecca5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 16 Apr 2024 22:31:53 +0100 Subject: [PATCH 06/10] Remove spurious indentation of HTTP headers It would be nice to make the headers and body more of a unit somehow, but this is an improvement at least. --- content/client-server-api/_index.md | 30 +++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/content/client-server-api/_index.md b/content/client-server-api/_index.md index ba5272755..4b60d11ea 100644 --- a/content/client-server-api/_index.md +++ b/content/client-server-api/_index.md @@ -546,8 +546,10 @@ request parameter. A client should first make a request with no `auth` parameter. The homeserver returns an HTTP 401 response, with a JSON body, as follows: - HTTP/1.1 401 Unauthorized - Content-Type: application/json +``` +HTTP/1.1 401 Unauthorized +Content-Type: application/json +``` ```json { @@ -590,8 +592,10 @@ given. It also contains other keys dependent on the auth type being attempted. For example, if the client is attempting to complete auth type `example.type.foo`, it might submit something like this: - POST /_matrix/client/v3/endpoint HTTP/1.1 - Content-Type: application/json +``` +POST /_matrix/client/v3/endpoint HTTP/1.1 +Content-Type: application/json +``` ```json { @@ -611,8 +615,10 @@ along with the same object as when no authentication was attempted, with the addition of the `completed` key which is an array of auth types the client has completed successfully: - HTTP/1.1 401 Unauthorized - Content-Type: application/json +``` +HTTP/1.1 401 Unauthorized +Content-Type: application/json +``` ```json { @@ -643,8 +649,10 @@ but the client may make a second attempt, it returns the same HTTP status 401 response as above, with the addition of the standard `errcode` and `error` fields describing the error. For example: - HTTP/1.1 401 Unauthorized - Content-Type: application/json +``` +HTTP/1.1 401 Unauthorized +Content-Type: application/json +``` ```json { @@ -671,8 +679,10 @@ status 401 response as above, with the addition of the standard If the request fails for a reason other than authentication, the server returns an error message in the standard format. For example: - HTTP/1.1 400 Bad request - Content-Type: application/json +``` +HTTP/1.1 400 Bad request +Content-Type: application/json +``` ```json { From 7d17e0761e688a5711499dabfd76bd094a714d89 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 30 Apr 2024 19:00:11 +0100 Subject: [PATCH 07/10] Define `m.login.terms` authentication --- content/client-server-api/_index.md | 127 ++++++++++++++++++ .../definitions/m.login.terms_params.yaml | 82 +++++++++++ 2 files changed, 209 insertions(+) create mode 100644 data/api/client-server/definitions/m.login.terms_params.yaml diff --git a/content/client-server-api/_index.md b/content/client-server-api/_index.md index 4b60d11ea..ac0d9a008 100644 --- a/content/client-server-api/_index.md +++ b/content/client-server-api/_index.md @@ -980,6 +980,133 @@ in the registration process that their token has expired. {{% http-api spec="client-server" api="registration_tokens" %}} +##### Terms of service at registration + +{{% added-in v="1.11" %}} + +| Type | Description | +|--------------------------|-----------------------------------------------------------------------| +| `m.login.terms` | Authentication requires the user to accept a set of terms of service. | + +{{% boxes/note %}} +The `m.login.terms` authentication type is only valid on the +[`/register`](#post_matrixclientv3register) endpoint. +{{% /boxes/note %}} + +This authentication type is used when the homeserver requires new users to +accept a given set of policy douments, such as a terms of service and a privacy +policy. There may be many different types of documents, all of which are +versioned and presented in (potentially) multiple languages. + +When the server requires the user to accept some terms, it does so by returning +a 401 response to the `/register` request, where the response body includes +`m.login.terms` in the `flows` list, and the `m.login.terms` property in the +`params` object has the structure [shown below](#definition-mloginterms-params). + +If a client encounters an invalid parameter, registration should stop with an +error presented to the user. + +The client should present the user with a checkbox to accept each policy, +including a link to the provided url. Once the user has done so, the client +submits an `auth` dict as follows, to indicate that all of the policies have +been accepted: + +```json +{ + "type": "m.login.terms", + "session": "" +} +``` + +The server is expected to track which document versions it presented to the +user during registration, if applicable. + + +**Example** + +1. A client might submit a registration request as follows: + + ``` + POST /_matrix/client/v3/register + ``` + ```json + { + "username": "cheeky_monkey", + "password": "ilovebananas" + } + ``` + +2. The server requires the user to accept some terms of service before + registration, so returns the following response: + + ``` + HTTP/1.1 401 Unauthorized + Content-Type: application/json + ``` + ```json + { + "flows": [ + { "stages": [ "m.login.terms" ] } + ], + "params": { + "m.login.terms": { + "policies": { + "terms_of_service": { + "version": "1.2", + "en": { + "name": "Terms of Service", + "url": "https://example.org/somewhere/terms-1.2-en.html" + }, + "fr": { + "name": "Conditions d'utilisation", + "url": "https://example.org/somewhere/terms-1.2-fr.html" + } + } + } + } + }, + "session": "kasgjaelkgj" + } + ``` + +3. The client presents the list of documents to the user, inviting them to + accept the polices. + +4. The client repeats the registration request, confirming that the user has + accepted the documents: + ``` + POST /_matrix/client/v3/register + ``` + ```json + { + "username": "cheeky_monkey", + "password": "ilovebananas", + "auth": { + "type": "m.login.terms", + "session": "kasgjaelkgj" + } + } + ``` + +5. All authentication steps have now completed, so the request is successful: + ``` + HTTP/1.1 200 OK + Content-Type: application/json + ``` + ```json + { + "access_token": "abc123", + "device_id": "GHTYAJCE", + "user_id": "@cheeky_monkey:matrix.org" + } + ``` + +{{% definition path="api/client-server/definitions/m.login.terms_params" %}} + + + + + #### Fallback Clients cannot be expected to be able to know how to process every diff --git a/data/api/client-server/definitions/m.login.terms_params.yaml b/data/api/client-server/definitions/m.login.terms_params.yaml new file mode 100644 index 000000000..67001c189 --- /dev/null +++ b/data/api/client-server/definitions/m.login.terms_params.yaml @@ -0,0 +1,82 @@ +# Copyright 2024 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +type: object +title: m.login.terms params +description: Schema for `m.login.terms` entry in the `params` object in a User-Interactive Authentication response. +required: ['policies'] +properties: + policies: + type: object + description: | + A map from "Policy ID" to the current definition of this policy document. The Policy ID is a unique + identifier for a given policy document, using the [Opaque Identifier Grammar](/appendices/#opaque-identifiers). + additionalProperties: + type: object + title: Policy Definition + required: [version] + properties: + version: + type: string + description: | + The version of this policy document. This is provided as a convenience for the client, + and uses the [Opaque Identifier Grammar](/appendices/#opaque-identifiers). + additionalProperties: + type: object + title: Policy Translation + required: [name, url] + description: | + Map from language codes to details of the document in that language. + Language codes SHOULD be formatted as per [Section 2.2 of RFC + 5646](https://datatracker.ietf.org/doc/html/rfc5646#section-2.2), + though some implementations may use an underscore instead of dash + (for example, `en_US` instead of `en-US`). + properties: + name: + type: string + description: | + The name of this document, in the appropriate language. An + arbitrary string with no specified maximum length. + url: + type: string + description: | + A link to the text of this document, in the appropriate + language. MUST be a valid URI with scheme `https://` or + `http://`. Insecure HTTP is discouraged. +example: { + "policies": { + "terms_of_service": { + "version": "1.2", + "en": { + "name": "Terms of Service", + "url": "https://example.org/somewhere/terms-1.2-en.html" + }, + "fr": { + "name": "Conditions d'utilisation", + "url": "https://example.org/somewhere/terms-1.2-fr.html" + } + }, + "privacy_policy": { + "version": "1.2", + "en": { + "name": "Privacy Policy", + "url": "https://example.org/somewhere/privacy-1.2-en.html" + }, + "fr": { + "name": "Politique de confidentialité", + "url": "https://example.org/somewhere/privacy-1.2-fr.html" + } + } + } +} \ No newline at end of file From c118d7d087419579f317732f99afa30b92f2e15b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 30 Apr 2024 19:02:35 +0100 Subject: [PATCH 08/10] Changelog --- changelogs/client_server/newsfragments/1812.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/client_server/newsfragments/1812.feature diff --git a/changelogs/client_server/newsfragments/1812.feature b/changelogs/client_server/newsfragments/1812.feature new file mode 100644 index 000000000..baa9aa7d6 --- /dev/null +++ b/changelogs/client_server/newsfragments/1812.feature @@ -0,0 +1 @@ +Specify terms of services at registration, as per [MSC1692](https://github.com/matrix-org/matrix-spec-proposals/pull/1692). From 90c5e30093cc0ef78a11d9b273c8d89c6b891813 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 30 Apr 2024 19:11:38 +0100 Subject: [PATCH 09/10] minor edits --- content/client-server-api/_index.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/content/client-server-api/_index.md b/content/client-server-api/_index.md index ac0d9a008..057346933 100644 --- a/content/client-server-api/_index.md +++ b/content/client-server-api/_index.md @@ -1008,8 +1008,8 @@ error presented to the user. The client should present the user with a checkbox to accept each policy, including a link to the provided url. Once the user has done so, the client -submits an `auth` dict as follows, to indicate that all of the policies have -been accepted: +submits an `auth` dict with just the `type` and `session`, as follows, to +indicate that all of the policies have been accepted: ```json { @@ -1103,10 +1103,6 @@ user during registration, if applicable. {{% definition path="api/client-server/definitions/m.login.terms_params" %}} - - - - #### Fallback Clients cannot be expected to be able to know how to process every From 1654236313332c487fecfdb5efcb6ea6bce3a408 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 8 May 2024 14:49:48 +0100 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Hubert Chathi --- content/client-server-api/_index.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/content/client-server-api/_index.md b/content/client-server-api/_index.md index 057346933..b0ce72896 100644 --- a/content/client-server-api/_index.md +++ b/content/client-server-api/_index.md @@ -984,9 +984,9 @@ in the registration process that their token has expired. {{% added-in v="1.11" %}} -| Type | Description | -|--------------------------|-----------------------------------------------------------------------| -| `m.login.terms` | Authentication requires the user to accept a set of terms of service. | +| Type | Description | +|--------------------------|--------------------------------------------------------------------------| +| `m.login.terms` | Authentication requires the user to accept a set of policy documents. | {{% boxes/note %}} The `m.login.terms` authentication type is only valid on the @@ -994,7 +994,7 @@ The `m.login.terms` authentication type is only valid on the {{% /boxes/note %}} This authentication type is used when the homeserver requires new users to -accept a given set of policy douments, such as a terms of service and a privacy +accept a given set of policy documents, such as a terms of service and a privacy policy. There may be many different types of documents, all of which are versioned and presented in (potentially) multiple languages. @@ -1007,7 +1007,7 @@ If a client encounters an invalid parameter, registration should stop with an error presented to the user. The client should present the user with a checkbox to accept each policy, -including a link to the provided url. Once the user has done so, the client +including a link to the provided URL. Once the user has done so, the client submits an `auth` dict with just the `type` and `session`, as follows, to indicate that all of the policies have been accepted: