GNOME Bugzilla – Bug 774033
API to disable using cached credentials for a particular SoupMessage
Last modified: 2016-12-03 15:10:45 UTC
In WebKit we have a load option that disables using credential storage. We are currently disabling auth manager feature for the message, but that also prevents the authenticate signal from being emitted. What we really want is that credentials stored are not used and the authenticate signal is always emitted. Maybe the easiest API would be to add new SoupMessageFlags SOUP_MESSAGE_IGNORE_CACHED_CREDENTIALS and check that flag from SoupAuthManager.
Created attachment 339237 [details] [review] Add API to disable using cached credentials for a particular SoupMessage In the end I added SOUP_MESSAGE_DO_NOT_USE_AUTH_CACHE, because that message not only shouldn't read the cached credentials, but it shouldn't cache the credentials either.
This patch depends on patch attached to bug #774031
Comment on attachment 339237 [details] [review] Add API to disable using cached credentials for a particular SoupMessage >+ /* If the message already has a ready auth, use that instead */ >+ auth = soup_message_get_auth (msg); >+ if (auth && soup_auth_is_ready (auth, msg)) >+ return auth; >+ >+ if (proxy) >+ return priv->proxy_auth; First line needs to be "auth = proxy ? soup_message_get_proxy_auth (msg) : soup_message_get_auth (msg)", right? But then, at that point, maybe you should just split the proxy version into a separate function. >+ soup_message_set_auth (msg, soup_auth_is_ready (auth, msg) ? auth : NULL); I don't love that we're calling this from auth_got_headers()... it means that we end up modifying the request headers in the middle of processing the response. Maybe the fix is that soup_message_set_auth() shouldn't modify the headers right away, but instead we should wait until soup_message_starting() to change them? (Then you can also simplify soup_message_set_auth() to not need to deal with updating the header when re-setting the same auth.) Alternatively, maybe only call soup_message_set_auth() here if DO_NOT_USE_AUTH_CACHE is set? That's guaranteed to not break any pre-existing code. >@@ -1311,22 +1311,25 @@ soup_message_set_auth (SoupMessage *msg, SoupAuth *auth) presumably any change you make to soup_message_set_auth() needs to be made to soup_message_set_proxy_auth() too? >+ * @SOUP_MESSAGE_DO_NOT_USE_AUTH_CACHE: The #SoupAuthManager should not use >+ * the credentials cache for this message, neither to use cached credentials >+ * to automatically authenticate this message nor to cache the credentials >+ * after the message is successfully authenticated. Since 2.58 Clarify that this applies to both server and proxy auth, and note how it is different from disabling the auth manager for the message.
Created attachment 339933 [details] [review] Updated patch
(In reply to Dan Winship from comment #3) > Comment on attachment 339237 [details] [review] [review] > Add API to disable using cached credentials for a particular SoupMessage > > >+ /* If the message already has a ready auth, use that instead */ > >+ auth = soup_message_get_auth (msg); > >+ if (auth && soup_auth_is_ready (auth, msg)) > >+ return auth; > >+ > >+ if (proxy) > >+ return priv->proxy_auth; > > First line needs to be "auth = proxy ? soup_message_get_proxy_auth (msg) : > soup_message_get_auth (msg)", right? But then, at that point, maybe you > should just split the proxy version into a separate function. Right, I split it in two functions in the end. > >+ soup_message_set_auth (msg, soup_auth_is_ready (auth, msg) ? auth : NULL); > > I don't love that we're calling this from auth_got_headers()... it means > that we end up modifying the request headers in the middle of processing the > response. > > Maybe the fix is that soup_message_set_auth() shouldn't modify the headers > right away, but instead we should wait until soup_message_starting() to > change them? (Then you can also simplify soup_message_set_auth() to not need > to deal with updating the header when re-setting the same auth.) I did that, soup_message_set_auth() just keeps the SoupAuth, and SoupAuthManager updates the headers on starting and also on got_body, but only when using SOUP_MESSAGE_DO_NOT_USE_AUTH_CACHE and right before the message is requeued. > Alternatively, maybe only call soup_message_set_auth() here if > DO_NOT_USE_AUTH_CACHE is set? That's guaranteed to not break any > pre-existing code. Now it doesn't update the headers there in any case. > >@@ -1311,22 +1311,25 @@ soup_message_set_auth (SoupMessage *msg, SoupAuth *auth) > > presumably any change you make to soup_message_set_auth() needs to be made > to soup_message_set_proxy_auth() too? Right, I forgot about it. > >+ * @SOUP_MESSAGE_DO_NOT_USE_AUTH_CACHE: The #SoupAuthManager should not use > >+ * the credentials cache for this message, neither to use cached credentials > >+ * to automatically authenticate this message nor to cache the credentials > >+ * after the message is successfully authenticated. Since 2.58 > > Clarify that this applies to both server and proxy auth, and note how it is > different from disabling the auth manager for the message. Done.
pushed with a fixup to make ntlm-test not crash