- From: Darrel Miller via Datatracker <noreply@ietf.org>
- Date: Sun, 02 Feb 2025 16:11:31 -0800
- To: <ietf-http-wg@w3.org>
- Cc: draft-kowalik-domainconnect.all@ietf.org
Reviewer: Darrel Miller
Review result: On the Right Track
What follows is a early review on behalf of the HTTP Directorate. In general
this document clearly describes the problem being addressed and does a good job
of describing the proposed solution. Ideally, the solution would contain less
prescriptive path requirements, and leverage more indirect extensibility
mechanisms that puts less URL constraints on the implementer. e.g. linksets and
link relations.
I do think there needs to be more text written to explain how the template
variable substitution mechanism works as there are many places for ambiguity to
creep in this type of processing.
Here are bunch of specific suggestions by section:
## 4.1. Templates
Suggest replacing
> The individual records in a template may be identified by a groupId.
with
The individual records in a template may be categorized by a groupId.
# 6. DNS Provider Discovery
The URL prefix should clarify that it can contain path segments.
Why does `urlControlPanel` use % for identifying variables. Why not use {}
considering it is a variable in a URL?
# 7.2.1. Query Supported Template
> Returning a status of 200 without a body indicates the template is
supported.
Why not 204?
# 7.2.2.1. Apply Template URL
> {urlSyncUX}/v2/domainTemplates/providers/{providerId}/services
/{serviceId}/apply?[properties]
I wonder if defining a link relation and a templated link would be a better
solution here rather than requiring a strict path hierarchy in the UX website.
In fact, a number of the properties in the settings resource are links and may
be more flexibly represented using a linkset
https://www.rfc-editor.org/rfc/rfc9264.html
Also, if you want to represent an unknown set of key-value pairs in a query
parameter you can use the following URI Template syntax
.../apply{?properties}
# 7.2.2.3. Same Browser Window
It is not clear how the redirect_uri is provided as the properties have not yet
been described,
It might be good to qualify this statement
> the DNS Provider must redirect the browser to a return URL (redirect_uri).
to say
> the DNS Provider must redirect the browser to a return URL (redirect_uri)
using a query parameter.
Similarly, the use of syncRedirectDomain without clarifying where this is
communicated is confusing.
> To prevent an open redirect, unless the request is digitally signed
> the redirect_uri must be within the domains specified in the template
> in syncRedirectDomain.
Suggest:
> must be within the domains specified in the template
property syncRedirectDomain.
# 7.2.2.4. Parameters/properties
For the "Name/Value Pairs", using a prefix such as "t-" would safely namespace
these parameter names to allow future addition of properties without concerns
about clashing with deployed template parameters.
This URL seems to be unnecessarily using the HTML entity for ampersand in the
URL
> https://web-connect.dnsprovider.example/v2/domainTemplates/providers/
exampleservice.example/services/template1/apply?domain=example.com
&IP=192.168.42.42&RANDOMTEXT=shm%3A1542108821%3AHello
# 7.3.3. OAuth Flow: Getting an Authorization Code
The query strings do not seem to be correctly formatted
> scope=t1+t2=example.com=sub1,sub2
I think should be
> scope=t1%20t2&domain=example.com&host=sub1,sub2
# 7.3.4. OAuth Flow: Requesting an Access Token
Rather than hard coding the token path to `/v2/oauth/access_token` the token
acquisition URL could be another URL made available via the settings URLs.
Is it really necessary to redefine the parameters of the token API endpoint?
Could this not be deferred to OAuth2?
# 7.3.6. OAuth Flow: Apply Template to Domain.
Considering the async flow must use POST, is there an advantage to allow either
query parameters or JSON post body? Do service providers need to support a
combination of query parameters and JSON body? If there are duplicates, which
takes priority? Can the name/value pairs be split between body and query? If
groupId exists in both, are the values combined?
The 409 response body should consider using the HTTP Problem Details media type
(RFC9457).
# 7.3.7. OAuth Flow: Revert Template
The URI Template to make domain required and host optional looks like this:
> {urlAPI}/v2/domainTemplates/providers/{providerId}/services
/{serviceId}/revert?domain={domain}{&host}
# 8.2. Template Definition
Consider defining the template JSON data structure as a media type.
# 8.3. Template Record
Is the * part of this syntax or just a bullet?
> *%host%:
# 9.9.2. Variable Values
This query string example seems wrong
> "domain=example.com="
It would be good to have some explicit guidance on how variable substitution
should occur. What happens when input values have whitespace? Can input values
only be strings? If not, how are other data types handled? This is especially
important considering sometimes input values can be provided by query string
and sometimes by JSON body.
Happy to provide more details if my feedback is not clear.
Darrel
Received on Monday, 3 February 2025 00:11:36 UTC