[cl-openid-devel] Re: cl-openid: API design, code finalization and nearest future plans

[Maciek Pasternacki]

Hello,

Since this is not personal message, but design discussion, Cc: to
development list so that anyone interested can chime in.

Anton Vodonosov <avodonosov at yandex.ru> writes:

> The SoC programs comes to its end. It is time to finalize the code
> and write documentation. First of all, lets discuss what remains
> to be done in the code:
>
>   - what to make the public interface of the library
>   - what to keep private
>   - what to left as a responsibility of the library user
>
> In short, I think current design is adequate and requires only small
> refinement. Below is my view of this.

As I wrote in yesterday's report, there's one more thing I decided to
change.  I'd wish to turn from alists representing messages to
OPENID-MESSAGE structure (it’s just a struct, since there’s no need
for OOP here).  The structure would be a thin layer atop parameter
alist, plus accessors.  This may make the code cleaner by use of
clearly named accessors instead of sprinkling code with AGETs.  Also,
commonly used lookups (such as namespace check to determine protocol
version) can be moved to struct fields, which would make many alist
lookups unnecessary.

This is actually not a major change, but one I see really useful for
cleaning up the code.

> For the Relying Party two functions are intended for most of
> the library users:
>
> (initiate-authentication relying-party given-id return-to
>                          realm &key immediate-p)
> Performs dicsovery, association, creates and stores authentication
> transaction object in the relying-party under unique key; returns URI
> to redirect user browser.
>
> (handle-indirect-response relying-party requested-url parameters)
> ;; I am not sure here, maybe it is sufficient to have just one
> ;; parameter - requested-url including query part.
> Lookups the authentication transaction object in the relying-party
> by unique key in the requested-url (or parameters). Verifies the assertion.
> Returns: authenticated identity, authentication failture or setup-needed.
>
> Here relying-party parameter is an object encapsulating current *ids*,
> *associations* and *nonces*.

I agree.  That’s also pretty much the same API as python-openid’s
(which is a bit more convoluted because it’s more flexible; that’s
what we may want to add later).  Core of the script I used for testing
the Provider (the general request workflow, which I called from
ipython’s command line) looks like this:

,----
| c = openid.consumer.consumer.Consumer(
|     {}, openid.store.memstore.MemoryStore() )
| r = c.begin('http://common-lisp.net/project/cl-openid/')
| 
| redirection = r.redirectURL('http://example.com/', 'http://example.com/', immediate=True)
| 
| url = # paste return url from browser
| q = {}
| for k,v in cgi.parse_qsl(urlparse.urlparse(url)[4]):
|     q[k] = v.decode('utf-8')
| 
| i = c.complete(q, 'http://example.com/')
`----

Parameter parsing into a dictionary is user’s responsibility.

> In addition to this we may provide functions that more or less correspond 
> to the basic protocol operations described in the spec:
>
>         discover 
>         associate
>         request-authentication-uri
>         veryfy-assertion

I think I’ll create either a class, or a structure (didn’t decided yet
whether OO would be needed) instead of an “id” alist.  These parts of
API may be exported, and it would be glue between “id” and RP object.

I didn’t settle yet on a better name for what is currently “id” alist.

> And perhaps even
>
>         kv-encode
>         parse-kv
>         btwoc
>         base64-btwoc

I think those may stay internal, and some MESSAGE struct API would be
exported.  I think the rule of thumb may be to export things that seem
actually usable by someone wishing to use the library as a whole, not
only some parts of it as building blocks, and then consider exporting
other functions or features when someone asks for it on development
list.

> Creating user interface (login form, result page, etc.) and creation
> of web server handlers is responsibility of the library user.

I agree.  This way, all the parts labeled “Hunchentoot-specific” will
be provided as example implementations, not as a part of code.

These will actually need to be tailored to specific app, so providing
these parts as a code will not be actually useful.

> For the provider we offer protocol primitives to the library user:
>
>     (create-asssociation provider parameters ssl-p)
>     (check-authentication provider parameters)
>     (error-response ...)
>     All these return a string to be used as a response to 
>     direct HTTP request.
>
>     (positive-assertion-uri provider setup-parameters)
>     (negative-assertion-uri provider setup-parameters)
>     Both return URL to redirect user's browser to.
>
> The PROVIDER parameter here is an object encapsulating all the current
> provider-related globals.

So, INDIRECT-RESPONSE would go to an example, INDIRECT-RESPONSE-URI,
ERROR-RESPONSE and DIRECT-RESPONSE will be exit points, and entry
points will be registered callbacks.  I have to decide whether they’d
be slots containing functions, or specialized methods.

I’m not sure whether to expose CREATE-ASSOCIATION.  I’d rather lean to
exposing association preferences via provider object.

> Library user is responsible for creation of web server handler for the
> OP endpoint URI, based on example we provide (current handle-openid-provider-request
> function). This handler will dispatch to a necessary protocol
> primitive based on the "openid.mode" HTTP request parameter. 
> User handles "checkid_immediate" and "checkid_setup" as he wants.
>
> Note, this design, in particular, eliminates server portability
> question.

This is great idea, something I’ve started to consider lately after
looking at python-openid, but it didn’t take enough shape to be posted
yet.  Let’s go for it.

For the provider part, I’ll also look into python-openid for
inspiration, its API seems good, flexible and it is tested.

> Please provide your comments. In your blog you mentioned some MESSAGE
> object and I have impression that you are planing more complex
> refractoring.

Not really.  The OpenID protocol is concentrated on exchanging
messages.  The messages take different forms, external encodings and
transmission channels, but they’re essentially same thing all over.
Protocol draws a distinct line between logic (behaviour) and data
(message encoding, decoding, handling).

In current code, it is not so.  The message handling code is mixed
with logic, and done with general functions.  What I want to do is to
separate message-handling code to its own file, with clearly named
functions and accessors.

Consider the fragment from auth-request.lisp:

,----
| (let ((response (direct-request (aget :op-endpoint-url id)
|                                 (acons "openid.mode" "check_authentication"
|                                         (remove "openid.mode" parameters
|                                                               :key #'car
|                                                               :test #'string=)))))
|   …)
`----

Protocol logic is mixed with low-level message mangling.  I want it to
look more like this:

,----
| (let ((response (direct-request (endpoint-url auth-process)
|                                 (copy-message response
|                                               :mode "check_authentication"))))
|   …)
`----

This should also improve testability – it should be easy to write
tests for internal message API, and it would ensure there are no
alist-mangling-level bugs in the conversation implementation, so only
the behaviour would need to be tested there.

> After discussing API and refactoring, I hope we finish the coding ASAP
> and start documentation.
>
> As a program mentor, I postpone http-client and storage backend
> portability, until we will complete documentation and release
> a library, even despite these portability issues looks simple.
> I hope we will produce complete library by the program finish,
> and undocumented code may be qualified more like as unfinished work,
> then as complete library.

Agreed.

> Good example is Edi Weit's documentation and I hope we will provide
> something similar: short summary, links to specifications, breaf of
> protocol (very short but clean, for details users may read specs),
> documentation for all exported symbols, function parameters,
> installation and dependencies, etc.

We can even start from Edi’s DOCUMENTATION-TEMPLATE output.  I was
even thinking for a while about hacking it to be able to actually use
it for generating final doc, not only the template, but it would be
too much work to do right now.

> We need at least two examples: for RP and OP, based on
> handle-openid-request and handle-openid-provider-request.
> Samples must be simple, short, self-contained, fully functioning,
> single file working web sites. No portability over web server
> is necessary, to avoid obscuring open-id protocol and library API.
> If some function is used in both - duplicate, to keep sample
> single-file.
>
> Every protocol related invocation must be commented, to make
> users not familiar with open-id grasp the protocol idea from
> the code.

I will probably copy and polish a bit the Hunchentoot-specific parts
of relying-party.lisp and provider.lisp – most of example code is
already done.  I agree about the form.

> I feel we are short of time.

I think there is enough time to provide complete package with docs, I
think (without optional features, though).  There is not really much
refactoring to do, and final week can be used for polishing
documentation, bugfixing and testing.  I’ll add the docstrings and
exports as I go with refactoring.

Regards,
Maciej.