[Captive-portals] Review of draft-ietf-capport-api-02

[Kyle Larose <[e-mail]>]

Tommy and Darshak,

Thanks for the update. Changing host to client makes sense. I hadn't
considered the NTP case either; it's pretty important!

I've done a re-read of the document. It is still nice and simple. Good job!

I have some comments.

In section 3, the document outlines the steps of interaction between a
client and the captive portal service. It says:

>   3.  Enforcement, in which the enforcement device in the network
>        blocks disallowed traffic, and sends ICMP messages to let clients
>        know they are blocked by the captive portal

The architecture now refers to the ICMP message as a the "Captive
Portal Signal" (since we haven't decided on a concrete
implementation). We should be consistent here.

It may also be worth noting that the signal is optional, though it may
be easier to read if we don't bring that up in the API document.

In Section 4.2, the data structure defines the following json field:

>   o  "vendor-info-url" (optional, string): provides the URL of a
>      webpage or site on which the operator of the network has
>      information that it wishes to share with the user (e.g. store
>      info, maps, flight status, or entertainment).

We don't provide any concrete examples or guidance on how to use this.
Is it something to user-portal-url would refer to? Should it be shown
alongside the user-portal-url? Would some text along the lines of:

    "The client MAY notify the user of this url/provide a link/etc..."

work? Or is that getting too into the UX details?

It also defines:

>  o  "bytes-remaining" (optional, integer): indicates the number of
>      bytes remaining, after which the client will be in placed into a
>      captive state.

Should we be clear about whether this applies to bytes sent by the
client, bytes received by the client, or both (i.e. the sum). We could
also get into the details of at which layer the enforcement device is
doing the counting, but that may be too much detail.

A general note regard the data structure:  I'm not sure if we've
brought this up in the past. Do we want to version the data structure?
Or would future extensions simply add keys whose presence implies the
new version?

In section 4.3, the example HTTP request starts with:

>   GET /captive-portal/api/X54PD

Nit: don't we need an HTTP version at the end there?

That's all for now.

Have fun in Prague,

Kyle