Re: Httpdir early review of draft-kowalik-domainconnect-00

Hi Darrel,

Most of the issues were addressed in -01 of the draft. You may want to 
review if anything needs further refinement.

This will be covered in the WG session tomorrow, but you may use the 
slides [1] (18-25) to navigate through the items and implemented changes.

  [1] 
https://datatracker.ietf.org/meeting/124/materials/slides-124-dconn-ietf-124-domain-connect-issue-summary-01


Kind Regards,

Pawel

On 04.02.25 17:37, Pawel Kowalik wrote:
>
> Thank you Darrell, more notes / questions below.
>
> Kind Regards,
>
> Pawel
>
> On 03.02.25 01:11, Darrel Miller via Datatracker wrote:
>> 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.
>
> OK, I will take a look into that.
>
>> 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.
>
> I think "categorised" is not a perfect match, as groupId actually 
> makes a group of RRs addressable and applicable separately from other 
> parts of the template.
>
> Would "may be assigned to a group identified by a groupId" work better?
>
>>   # 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?
>
> A fair consideration. I think the guiding principle here was to keep 
> the same language for variable substitution across the whole 
> specification.
>
> But, accounting that here there is different consumer for this part 
> compared to the template, this may be a valid suggestion to use {}. 
> Thanks.
>
>> # 7.2.1. Query Supported Template
>>
>>>   Returning a status of 200 without a body indicates the template is
>>   supported.
>>
>> Why not 204?
>
> To keep the same status code no matter if the representation is not 
> given (empty) or if there is any of the other representations returned 
> (only version or full template).
>
> The clients not interested in the content may just process 200 response.
>
> The description of 204 in RFC7231 does not exclude usage for GET, but 
> suggests there is some "action" so like more applicability to PUT or 
> DELETE. In this sense 204 didn't appear as an obvious match for this 
> use case.
>
>> # 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

>
> I will have to take a look into this. Is the suggestion to go in the 
> similar direction as urlControlPanel with path template with 
> {providerId} and {serviceId} included?
>
> I would need to take a look into rfc9264 how useful it might be for 
> this use-case.
>
>> 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}
> OK, thanks. This can be applied.
>> # 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,
>
> OK, will take a look into this.
>
>> 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.
> ok.
>> 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.
> ok
>>   # 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.
>
> OK, this is a fair point about potential conflicts between template 
> variables and current and future built-in parameters.
>
> Do you suggest to prefix only in the application, so to "t-foo" would 
> be actually interpreted as "foo" in the template, or to say that all 
> template-defined variables must always start with "t-" prefix?
>
>>   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
> Thx. Will check that.
>> # 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
>
> Thx. Will check that.
>
>> # 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?
>
> The point here was to profile OAuth2 to have a narrow always working 
> set of functionality. Likely this needs a second look into.
>
>> # 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?
>
> I think the body variant is not actually needed or used, but I would 
> have to check. This is a good catch.
>
>> The 409 response body should consider using the HTTP Problem Details media type
>> (RFC9457).
> ok
>> #  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}
> Thx. Will take over.
>> # 8.2. Template Definition
>>
>> Consider defining the template JSON data structure as a media type.
> ok
>> # 8.3. Template Record
>>
>> Is the * part of this syntax or just a bullet?
>>
>>>   *%host%:
> This is just a bullet. At least in TXT version it's rendered properly. 
> Same in HTML. Where did you faced the star "glued" to %host%?
>> # 9.9.2. Variable Values
>>
>> This query string example seems wrong
>>
>>>   "domain=example.com="
>
> I think some asciidoc to ietf conversions are broken. I will take a 
> look into that.
>
>> 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.
> Noted. Will take a look to clarify that.
>> Happy to provide more details if my feedback is not clear.
>>
>> Darrel
>>
>>