Re: [6/7] qmgr: Add support for Basic authentication.

[Jacek Caban]

Hi Hans,

On 06/08/15 14:32, Hans Leidekker wrote:
> static HRESULT WINAPI http_negotiate_BeginningTransaction(
>      IHttpNegotiate *iface, LPCWSTR url, LPCWSTR headers, DWORD reserved, LPWSTR *add_headers)
>  {
>      DLBindStatusCallback *callback = impl_from_IHttpNegotiate(iface);
> - FIXME("(%p)->(%s %s %u %p)\n", callback, debugstr_w(url), debugstr_w(headers), reserved, add_headers);
> - return E_NOTIMPL;
> - BackgroundCopyJobImpl *job = callback->file->owner;
> - WCHAR *str, *auth_header;
> - DWORD len;
> -
> - TRACE("(%p)->(%s %s %d %p)\n", callback, debugstr_w(url), debugstr_w(headers), reserved, add_headers);
> -
> - if ((auth_header = build_auth_header(&job->http_options.creds)))
> - {
> -
>    if (!job->http_options.headers)
> -
>    {
> -
>        *add_headers = auth_header;
> -
>        return S_OK;
> -
>    }
> -
>    else
> -
>    {
> -
>        len = strlenW(auth_header) + strlenW(job->http_options.headers);
> -
>        if (!(str = CoTaskMemAlloc((len + 1) * sizeof(WCHAR))))
> -
>        {
> -
>            CoTaskMemFree(auth_header);
> -
>            return E_OUTOFMEMORY;
> -
>        }
> -
>        strcpyW(str, auth_header);
> -
>        strcatW(str, job->http_options.headers);
> -
>        CoTaskMemFree(auth_header);
> -
>        *add_headers = str;
> -
>        return S_OK;
> -
>    }
> - }
> - else if (job->http_options.headers)
> - {
> -
>    if (!(*add_headers = co_strdupW(job->http_options.headers))) return E_OUTOFMEMORY;
> -
>    return S_OK;
> - }
> -
> - *add_headers = NULL;
> - return S_OK;

It doesn't sound like a good idea to manually handle that here. We
should most likely use IAuthenticate (although it's not yet supported in
urlmon).

Jacek

[6/7] qmgr: Add support for Basic authentication.

[Hans Leidekker]

Hi Jacek,
> Hi Hans,
>
> On 06/08/15 14:32, Hans Leidekker wrote:
> > static HRESULT WINAPI http_negotiate_BeginningTransaction(
> >      IHttpNegotiate *iface, LPCWSTR url, LPCWSTR headers, DWORD reserved, LPWSTR *add_headers)
> >  {
> >      DLBindStatusCallback *callback = impl_from_IHttpNegotiate(iface);
> > - FIXME("(%p)->(%s %s %u %p)\n", callback, debugstr_w(url), debugstr_w(headers), reserved, add_headers);
> > - return E_NOTIMPL;
> > - BackgroundCopyJobImpl *job = callback->file->owner;
> > - WCHAR *str, *auth_header;
> > - DWORD len;
> > -
> > - TRACE("(%p)->(%s %s %d %p)\n", callback, debugstr_w(url), debugstr_w(headers), reserved, add_headers);
> > -
> > - if ((auth_header = build_auth_header(&job->http_options.creds)))
> > - {
> > -
> >    if (!job->http_options.headers)
> > -
> >    {
> > -
> >        *add_headers = auth_header;
> > -
> >        return S_OK;
> > -
> >    }
> > -
> >    else
> > -
> >    {
> > -
> >        len = strlenW(auth_header) + strlenW(job->http_options.headers);
> > -
> >        if (!(str = CoTaskMemAlloc((len + 1) * sizeof(WCHAR))))
> > -
> >        {
> > -
> >            CoTaskMemFree(auth_header);
> > -
> >            return E_OUTOFMEMORY;
> > -
> >        }
> > -
> >        strcpyW(str, auth_header);
> > -
> >        strcatW(str, job->http_options.headers);
> > -
> >        CoTaskMemFree(auth_header);
> > -
> >        *add_headers = str;
> > -
> >        return S_OK;
> > -
> >    }
> > - }
> > - else if (job->http_options.headers)
> > - {
> > -
> >    if (!(*add_headers = co_strdupW(job->http_options.headers))) return E_OUTOFMEMORY;
> > -
> >    return S_OK;
> > - }
> > -
> > - *add_headers = NULL;
> > - return S_OK;
>
> It doesn't sound like a good idea to manually handle that here. We
> should most likely use IAuthenticate (although it's not yet supported in
> urlmon).

I considered that but it doesn't seem possible to select the authentication
target (server vs. proxy) or scheme with IAuthenticate.

[6/7] qmgr: Add support for Basic authentication.

[Jacek Caban]

On 06/08/15 16:44, Hans Leidekker wrote:
> Hi Jacek,
> > It doesn't sound like a good idea to manually handle that here. We
> > should most likely use IAuthenticate (although it's not yet supported in
> > urlmon).
>
> I considered that but it doesn't seem possible to select the authentication
> target (server vs. proxy) or scheme with IAuthenticate.

Quick research suggests that IAuthenticateEx allows that.

Jacek

[6/7] qmgr: Add support for Basic authentication.

[Hans Leidekker]

On Mon, 2015-06-08 at 16:51 +0200, Jacek Caban wrote:
> On 06/08/15 16:44, Hans Leidekker wrote:
> > Hi Jacek,
> > > It doesn't sound like a good idea to manually handle that here. We
> > > should most likely use IAuthenticate (although it's not yet supported in
> > > urlmon).
> >
> > I considered that but it doesn't seem possible to select the authentication
> > target (server vs. proxy) or scheme with IAuthenticate.
>
> Quick research suggests that IAuthenticateEx allows that.

Right, IAuthenticateEx allows specifying the target but it doesn't give the
same control over schemes that IBackgroundCopyJob2::SetCredentials does.

[6/7] qmgr: Add support for Basic authentication.

[Jacek Caban]

On 06/08/15 17:10, Hans Leidekker wrote:
> On Mon, 2015-06-08 at 16:51 +0200, Jacek Caban wrote:
> > On 06/08/15 16:44, Hans Leidekker wrote:
> > > Hi Jacek,
> > > > It doesn't sound like a good idea to manually handle that here. We
> > > > should most likely use IAuthenticate (although it's not yet supported in
> > > > urlmon).
> > >
> > > I considered that but it doesn't seem possible to select the authentication
> > > target (server vs. proxy) or scheme with IAuthenticate.
> >
> > Quick research suggests that IAuthenticateEx allows that.
>
> Right, IAuthenticateEx allows specifying the target but it doesn't give the
> same control over schemes that IBackgroundCopyJob2::SetCredentials does.

I think it does. AUTHENTICATEINFO passes requested scheme.

Jacek

[6/7] qmgr: Add support for Basic authentication.

[Hans Leidekker]

On Tue, 2015-06-09 at 12:34 +0200, Jacek Caban wrote:
> On 06/08/15 17:10, Hans Leidekker wrote:
> > On Mon, 2015-06-08 at 16:51 +0200, Jacek Caban wrote:
> > > On 06/08/15 16:44, Hans Leidekker wrote:
> > > > Hi Jacek,
> > > > > It doesn't sound like a good idea to manually handle that here. We
> > > > > should most likely use IAuthenticate (although it's not yet supported in
> > > > > urlmon).
> > > >
> > > > I considered that but it doesn't seem possible to select the authentication
> > > > target (server vs. proxy) or scheme with IAuthenticate.
> > >
> > > Quick research suggests that IAuthenticateEx allows that.
> >
> > Right, IAuthenticateEx allows specifying the target but it doesn't give the
> > same control over schemes that IBackgroundCopyJob2::SetCredentials does.
>
> I think it does. AUTHENTICATEINFO passes requested scheme.

AUTHENTICATEINFO allows specifying whether Basic authentication may be used but
doesn't give control over any other specific scheme.

I see no way to properly implement this with urlmon/wininet other than by inserting
headers manually. It occurred to me that IBackgroundCopyJob2::SetCredentials
maps cleanly to WinHttpSetCredentials and some googling revealed that native
does indeed use winhttp:

https://support.microsoft.com/en-us/kb/842773
https://msdn.microsoft.com/en-us/library/aa964260%28v=vs.85%29.aspx (remarks section).

[6/7] qmgr: Add support for Basic authentication.

[Jacek Caban]

On 06/09/15 12:50, Hans Leidekker wrote:
> I see no way to properly implement this with urlmon/wininet other than by inserting
> headers manually. It occurred to me that IBackgroundCopyJob2::SetCredentials
> maps cleanly to WinHttpSetCredentials and some googling revealed that native
> does indeed use winhttp

Interesting. Then winhtttp is the right way to do it.

Jacek