224749 - REUSABLE_CREDENTIALS and REUSABLE_CHALLENGE flags ignored in nsHttpChannel::setAuthorization

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Christopher Nebergall]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031101 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031101 Firebird/0.7+

 if ((!creds[0] || identFromURI) && challenge[0]) {
2369             nsCAutoString unused;
2370             rv = ParseChallenge(challenge, unused, getter_AddRefs(auth));
2371             if (NS_SUCCEEDED(rv)) {
2372                 nsISupports *sessionState = entry->mMetaData;
2373                 rv = auth->GenerateCredentials(this, challenge,
2374                                                header ==
nsHttp::Proxy_Authorization,
2375                                                ident.Domain(),
2376                                                ident.User(),
2377                                                ident.Password(),
2378                                                &sessionState,
2379                                                &mAuthContinuationState,
2380                                                getter_Copies(temp));
2381 
2382                 entry->mMetaData.swap(sessionState);
2383                 if (NS_SUCCEEDED(rv)) {
2384                     creds = temp.get();
2385 
2386                     //
2387                     // set the cached authorization credentials to
2388                     // those we used (in case we used the URI
2389                     // specified credentials)
2390                     rv = authCache->SetAuthEntry(host, port, path,
2391                                                  entry->Realm(),
2392                                                  creds, challenge,
2393                                                  ident, sessionState);
2394                 }
2395             }

I recommend that it be changed to act like the following code from 
nsHttpChannel::GetCredentialsForChallenge

2134         PRBool saveCreds
2135                 = (authFlags & nsIHttpAuthenticator::REUSABLE_CREDENTIALS);
2136         PRBool saveChallenge
2137                 = (authFlags & nsIHttpAuthenticator::REUSABLE_CHALLENGE);
2138 
2139         // create a cache entry.  we do this even though we don't yet know that
2140         // these credentials are valid b/c we need to avoid prompting the user
2141         // more than once in case the credentials are valid.
2142         //
2143         // if the credentials are not reusable, then we don't bother sticking
2144         // them in the auth cache.
2145         rv = authCache->SetAuthEntry(host, port, path.get(), realm.get(),
2146                                      saveCreds ? creds.get() : nsnull,
2147                                      saveChallenge ? challenge : nsnull,
2148                                      *ident, sessionState);

Reproducible: Always

Steps to Reproduce:




Credentials cached from this call are replayed to the server in
nsHttpChannel::SetAuthorizationHeader.  If you are using Kerberos
authentication, it causes a replay error on the server and a 401 which can make
an auth plugin to decline to be ran a second time because it believes it has
already failed.  This will make the client fail to other authentication types
without reason.

Comment 1

[Darin Fisher]

Attachment: v1.0 patch

Comment 2

[Darin Fisher]

looks like there is enough common code between SetAuthorizationHeader and
GetCredentialsForChallenge that it makes sense to factor it into a helper
function.  so, i have created a GenCredsAndSetEntry method for that.

Comment 3

[Darin Fisher]

Comment on attachment 135227 [details] [diff] [review]
v1.0 patch

cneberg: can you please review this patch to ensure that it does the right
thing.	thanks!

Comment 4

[Christopher Nebergall]

I can't get to CVS from work so I installed this patch on 1.6a along the patch 
from bug 180049 and everything worked fine with the kerberos plugin.  Thanks!

Comment 5

[Darin Fisher]

Comment on attachment 135227 [details] [diff] [review]
v1.0 patch

cneberg: thanks for verifying the patch!

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 135227 [details] [diff] [review]
v1.0 patch

So the point here is we need to look at the flags when deciding what to pass to
SetAuthEntry?  If so, sr=bzbarsky.

Sorry this took so long -- I was totally thrown off by all the unrelated
changes in this patch and thought I was missing something.

Comment 7

[Darin Fisher]

fixed on trunk for mozilla 1.6 beta