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

[Darshak Thakore <[e-mail]>]

Hi Kyle,
Thanks for the review and the comments.
Responses inline..

On 3/24/19, 7:51 AM, "Kyle Larose" <[e-mail]> wrote:

    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.

DT>>>  Agreed and good catch, will update that to refer to it as a generic "signals  the clients to know they are blocked.."
    
    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?

DT>>> This would be a good thing to discuss during the session. In general the idea was to not be prescriptive about what to do with the url but given that the main purpose of this optional url is to allow the user to navigate/see something maybe even before they got thru any kind of authentication, we may want to put some "recommendation" text.
    
    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.

DT>>> That question has been asked multiple times now so we certainly need some clarity.
    
    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?

DT>>> I guess future-proofing API best practices mandate a version to be there __
    
    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