[cl-openid-devel] Re: provider API

[Maciek Pasternacki]

Anton Vodonosov <avodonosov at yandex.ru> writes:

> What I mean, the user must create something like following code,
> based on the pattern we will provide in the example:
>
> (define-easy-handler (opendid-provider-endpoint
>                       "/openid-provider-endpoint")
>     ((mode :real-name "openid.mode")
>      (id :real-name "openid.identity"))
>   (let ((params (append (hunchentoot:post-parameters)
>                         (hunchentoot:get-parameters))))
>     (cond ((string= mode "associate")
>            (cl-openid:create-association *provider* 
>                                          params 
>                                          hunchentoot:ssl-p))
>           ((string= mode "checkid_immediate")
>            ;; here user's code cames, for example:
>            (if (and (string= (session-value 'cur-user)
>                              id)
>                     ;; BTW, it looks like we completelly 
>                     ;; missed return_to verification 
>                     ;; based on realm; see 9.2.1
>                     ;; we must provide some function(s) for this
>                     (cl-open-id:realm-is-good ...)) 
>                (hunchentoot:redirect (positive-assertion-uri *provider* 
>                                                              params))
>                (hunchentoot:redirect (negative-assertion-uri *provider* 
>                                                              params)))
>           ((string= mode "checkid_setup")
>              ;; here user's defined code too
>              ... )
>           ((string= mode "check_authentication")
>            (check-authentication *provider* params))
>           (t (error-response (format nil 
>                                      "Unknown openid.mode ~S" 
>                                      mode))))))
>
> I.e. no callbacks, user does not plug into our code, but wraps our
> code into his handler. I suspect this would be simpler for him.
> He must dispatch all the modes to predefined functions, except
> for "checkid_setup" and "checkid_immediate" that he handles
> himself.

I don't like this.  This way, we put too much of protocol-related code
in user's hands.  That's what higher-order functions are for; and
protocol implementation really belongs to our library.  I see it more
like this:

(defvar *op*
  (make-instance 'openid-provider
                 ;; true -> id_res, false -> setup_needed, error raised -> error
		 :immediate-callback #'(lambda (provider message)
					 (string= (session-value 'cur-user)
						  (claimed-id message)))

                 ;; Do own authentication (e.g. redirect to login page), finally redirect
		 :setup-callback #'(lambda (provider message)
				     ...)))

;; Or:
(defclass my-op (openid-provider) ()
  ())

(defmethod checkid-immediate ((op my-op) message)
  (string= (session-value 'cur-user) (claimed-id message)))

(defmethod checkid-setup ((op my-op) message)
  ...)

(defvar *op*
  (make-instance 'my-op ...))

; Maybe instead of subclassing, the EQL-specialized methods would be
; good enough?

;; Finally:
(define-easy-handler (openid-provider-endpoint "/openid-provider-endpoint") ()
  (multiple-value-bind (body status redirect-uri)
      (cl-openid:handle-provider-request *op* (append (get-parameters) (post-parameters)))
    (when redirect-uri
      (hunchentoot:redirect redirect-uri))
    (when status
      (setf (hunchentoot:return-code status)))
    body))

It's way simpler and less fragile.  There is no protocol
implementation mixed into HTTP handler.

> While writing this code sketch, I realized that not all the details
> are clear, e.g. glaring oversight is the return-to verification based
> on realm.

You're right, I missed it.  Just added a ticket to make sure it won't
get lost.

Regards,
Maciej.