On Tue, Nov 04, 2008 at 01:25:02PM +0100, Daniel Stenberg wrote:

Thanks for the comments, Daniel!

> On Mon, 3 Nov 2008, Josef Wolf wrote:

> >This version introduces CURLOPT_CREDENTIALFUNCTION to install the
> >callback. There are no functional changes as long as the callback is not
> >installed.
>
> I think it would make sense to also provide a CURLOPT_CREDENTIALDATA in the
> same sense all the other callbacks do, to provide a user pointer to the
> callback.

Yes..

> >But to make this work, the curl utility should be able to detect whether
> >it is running a test case, whether to install the callback or not, and
> >which credentials to provide. I guess there's no mechanism to handle this
> >in test cases yet?
>
> There is no such thing, but I would rather suggest that you'd write a
> little test case for the 5xx series of tests which all are libcurl-using
> tests that all have their own little test application that gets compiled
> and then runs. You can then write a little snippet that uses libcurl and
> installs the callback etc and use that to run the test(s).

Ah, I did not know that such things exist. I'll take a look at it.

> >Then we come to one more question (which Daniel already mentioned at the
> >very beginning): how should this be handled in the curl utility? Probably
> >it would make sense to introduce a command line switch to activate the
> >interactive callback.
>
> Yes I agree. But you I can also see how the curl tool could use the
> existing options to feed the callback data with if things match. Ie to use
> the new callback option to offer new functionality (auth based on the
> actual realm in the response header) and use the existing way to get that
> credential data. I mean, in addition to the optional prompting at the time
> when the callback gets called.

You mean to install the callback when --user or --proxy-user is supplied
without a password? AFAICS, those are the only cases where current
implementation would prompt for credentials. Changing any other
use-case without a new option could break existing (non-interactive)
scripts.

Then, there's the edge-case of http://user@host/. This case gives 401
in the current implementation. Dan stated this is by design. I am not
sure at all about this use case. For sure, prompting would break
existing scripts. OTOH, a 401 would break those scripts also. The
difference is that a prompt would let the script hang instead of
exit with an error code.

> Anyway, the actual implementation in the curl tool is somewhat
> unimportant and secondary to the libcurl change.

I need some playground for testing ;-) And I prefer a real example in
order to see whether the change actually improves things instead of
making them worse ;-) This does not mean that the hunk has to be
applied. There's always the option of dropping or reverting it :-)

> >BTW: I can't seem to find a way for properly determine the realm.
> > AFAICS, the realm is extracted only for Digest?
>
> Yes, as Digest is the only auth type that really need realm for its
> functionality there is no existing code that extracts it. This will
> have to be added to make this work properly.
>
> >Comments? Suggestions?
>
> Two comments/thoughts:
>
> A) A 401/407 response may indicate multiple authentication types. I'm
> not sure there is a guarantee that they will have the same realm.
> Thus it is of importance to keep track of only the one that is from
> the auth type of choice.

Hmmm, rfc2617 is not very explicit about that. And to be honest, I'm
not entirely sure I understand the RFC. The only thing I found is:

| The realm value
| (case-sensitive), in combination with the canonical root URL (the
| absoluteURI for the server whose abs_path is empty; see section 5.1.2
| of [2]) of the server being accessed, defines the protection space.

So AFAICS, the "protection space" is defined by the combination of
- protocol (e.g. https)
- host (e.g. www.host.com)
- realm (e.g. "My protected area")

| These realms allow the protected resources on a server to be
| partitioned into a set of protection spaces, each with its own
| authentication scheme and/or authorization database.

So the scheme(s) and authorization database should not change within
a specific proto/host/realm tuple.

| The realm value
| is a string, generally assigned by the origin server, which may have
| additional semantics specific to the authentication scheme. Note that
| there may be multiple challenges with the same auth-scheme but
| different realms.

I must admit, I don't really understand that last sentence. Does it
mean something like

  WWW-Authenticate: Basic realm="My protected area"
  WWW-Authenticate: Basic realm="Company protected"

would be legal in the _same_ response header? Which realm is to be
presented to the user when prompting for the credentials, then?

Anyway, it looks like you're right, the scheme should probably be added
to the tuple, because every challenge can have its own realm.

> B) Possibly a somewhat larger issue: what about the case when you use
> libcurl to get multiple pages from the same auth protected site
> that uses the same (or different) realms for the multiple resources
> your app requests? Are you then calling the callback on each
> occurrence to let the applicaton do any possible caching of the
> realm to credential mapping?

My intent was to cache credentials based on the above mentioned tuple
in the get_credentials() function (that is, within libcurl). I don't
see a point in letting every application invent its own caching wheel.

> This is perhaps most notable in cases
> where redirects are used (with closed connections in between) since
> then the callback could in fact get called multiple times when just
> a single resource is requested!

Hmmm, I am not sure I understand the scenario you try to point out here.
If we find the requested tuple in the cache, we can use the associated
credentials. If we don't find them, we should ask the application for
help. Am I missing something?
Received on 2008-11-04