Subject: [Captive-portals] AD review of draft-ietf-capport-api-06

From: Barry Leiba <[email-masked]>
Date: Fri, 24 Apr 2020 01:03:23 -0400

Thanks for a nice document.  Here’s my AD review prior to last call.  For a
few reasons, I think we need a revised I-D before going to last call.

— Section 2 —

   *  Captive Portal API Server: The server exposing the API's defined

Nit: no apostrophe should be in “APIs” (it’s not possessive).

This section also needs BCP 14 boilerplate (and the document needs
normative references to RFCs 2119 and 8174), as Sections 4 and 5 use BCP 14
key words.

— Section 3 —
I think 7710bis is an informative reference, not a normative one.

— Section 4.1 —

   valid certificate for the hostname in the URI (such as
   "example.com").

It’s a small point, and not wrong as written, but as there is an example
already given right above at the end of Section 4, I suggest referring to
it:

NEW
   valid certificate for the hostname in the URI
   ("example.org", in the example above).
END

— Section 5 —

In the bullet about seconds-remaining:

      The API server SHOULD include this value if the client is
      not captive (i.e. captive=false) and the client session is time-
      limited, and SHOULD omit this value for captive clients (i.e.
      captive=true).

What about captive=false and the client is not time-limited?  I think you
want, “and SHOULD omit this value value for captive clients (i.e.
captive=true) or when the client sessiin is not time-limited.”  Or, simpler
and better, “and SHOULD omit this value otherwise.”

And similarly for the next bullet about bytes-remaining.

— Section 6 —
The example violates the SHOULD in Section 5, “seconds-remaining”.  That’s
not a good idea.  It might be good to have two examples, one with
captive=true and one with captive=false.

   Upon receiving this information the client will provide this
   information to the user so that they may navigate the web portal

The client isn’t really going to provide the information to the user, is
it?  It will provide the information to another application, or will use
the information to allow the user to navigate the portal.  Can you rephrase
this so it doesn’t sound as if the client will say, “Here’s the portal URI;
go wild.” ?

— Section 8.1 —
Please double-check the registration template for media types in RFC 6838
and make sure this complies.

— Section 8.2 —

   New assignments for Captive Portal API Keys Registry will be
   administered by IANA through Expert Review [RFC8126].  The Designated
   Expert is expected to validate the existence of documentation
   describing new keys in a permanent publicly available specification.

How, then, is this not Specification Required?

—
Barry

---

From: Tommy Pauly <[email-masked]>
Date: Mon, 27 Apr 2020 10:55:08 -0700

Hi Barry,

Thanks for the review! Update made here:

 https://github.com/capport-wg/api/commit/27cd22fd8a8db696efb9772acd6c05d24a86c81d <https://github.com/capport-wg/api/commit/27cd22fd8a8db696efb9772acd6c05d24a86c81d>

This has been posted as draft-ietf-capport-api-07: https://tools.ietf.org/html/draft-ietf-capport-api-07 <https://tools.ietf.org/html/draft-ietf-capport-api-07>

> On Apr 23, 2020, at 10:03 PM, Barry Leiba <[email-masked]> wrote:
> 
> Thanks for a nice document.  Here’s my AD review prior to last call.  For a few reasons, I think we need a revised I-D before going to last call.
> 
> — Section 2 —
> 

Fixed
> 
> This section also needs BCP 14 boilerplate (and the document needs normative references to RFCs 2119 and 8174), as Sections 4 and 5 use BCP 14 key words.

Added
> I think 7710bis is an informative reference, not a normative one.

Changed to informative.
>    "example..com <http://example.com/>").
> 
> It’s a small point, and not wrong as written, but as there is an example already given right above at the end of Section 4, I suggest referring to it:
> 
> NEW
>    ("example.org <http://example.org/>", in the example above).
> END

Changed as suggested.
> What about captive=false and the client is not time-limited?  I think you want, “and SHOULD omit this value value for captive clients (i.e. captive=true) or when the client sessiin is not time-limited.”  Or, simpler and better, “and SHOULD omit this value otherwise.”
> 

Changed for both cases.
> 
> — Section 6 —
> The example violates the SHOULD in Section 5, “seconds-remaining”.  That’s not a good idea.  It might be good to have two examples, one with captive=true and one with captive=false.

Split the server response example into two.

> The client isn’t really going to provide the information to the user, is it?  It will provide the information to another application, or will use the information to allow the user to navigate the portal.  Can you rephrase this so it doesn’t sound as if the client will say, “Here’s the portal URI; go wild.” ?

The sentence now reads: "Upon receiving this information the client will use this information direct the user to the the web portal (as specified by the user-portal-url value) to enable access to the external network."

> 
> — Section 8.1 —
> Please double-check the registration template for media types in RFC 6838 and make sure this complies.

Updated for the missing "Fragment identifier considerations” field.
> How, then, is this not Specification Required?

Good point =) Updated to Specification Required, as that is what the text explains.

Best,
Tommy
> 
> — 
> Barry
> 
> _______________________________________________
> Captive-portals mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/captive-portals

---

From: Barry Leiba <[email-masked]>
Date: Mon, 27 Apr 2020 14:30:22 -0400

Thanks for the updates, Tommy... I have requested last call on -07,
and that will come out as soon as the Secretariat does their magic.

Barry

On Mon, Apr 27, 2020 at 1:55 PM Tommy Pauly <[email-masked]> wrote:
>  https://github.com/capport-wg/api/commit/27cd22fd8a8db696efb9772acd6c05d24a86c81d
>
> This has been posted as draft-ietf-capport-api-07: https://tools.ietf.org/html/draft-ietf-capport-api-07
>
>    "example..com").