260836 - RSS feeds which require HTTP authentication are rejected as invalid

Status: VERIFIED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Chris Adams]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US) AppleWebKit/85 (KHTML, like Gecko) OmniWeb/v558.48
Build Identifier: version 0.8 (20040913)

An RSS feed which requires HTTP authentication will be rejected as invalid rather than prompting 
the user to authenticate. 

Firefox has a similar bug with its Live Bookmarks which has already been reported (#259451) - in 
that case loading the URL manually will workaround the problem since the cached username/
password will be used when you refresh the live bookmark.

Reproducible: Always
Steps to Reproduce:
1. Attempt to subscribe to a feed which uses HTTP authentication
Actual Results:  
Thunderbird reports that the URL is not a valid feed

Expected Results:  
Prompt for a username and password

Comment 1

[Cornelius]

I ran across a similar problem when adding a live bookmark to an intranet site
that runs over https. The primary form of authentication is client certificates,
but the site also allows username/password authentication in certain cases.

After adding a love bookmark to the site, the live bookmarks feature would
attempt to visit the site, fail the certificate login (because it never gave me
the pin dialog) and drop back to username/password which would also fail. Then,
the next time I would visit the site, firefox would check and find that the
certificate authentication already failed once, so it goes straight into
username/password mode. I never got a second chance at the certificate login
unless I closed my browser and re-opened it.

Comment 2

[Cornelius]

Sorry, I meant the comment above in the firefox version of this bug.

Comment 3

[Marcia Knous [:marcia]]

could someone provide a test rss link which uses http authentication? thanks!

Comment 4

[Marcia Knous [:marcia]]

*** Bug 267836 has been marked as a duplicate of this bug. ***

Comment 5

[Marcia Knous [:marcia]]

seeing more http authentication bugs, so confirming for now.

Comment 6

[Marcia Knous [:marcia]]

*** Bug 270363 has been marked as a duplicate of this bug. ***

Comment 7

[Cornelius]

There are four rss test feeds here:

http://labs.silverorange.com/archives/2003/july/privaterss

Is that what you're looking for?

Comment 8

[sairuh (rarely reading bugmail)]

Cornelius, thanks for the test feed page!

yeah, this still fails for

a. RSS with HTTP Auth, but no SSL:
http://labs.silverorange.com/local/solabs/rsstest/httpauth/rss_with_auth.xml

b. RSS with both SSL and HTTP Auth:
https://secure3.silverorange.com/rsstest/httpauth/rss_with_ssl_and_auth.xml

no js console output.

however, this does work for RSS with SSL, but no HTTP authentication (recently
fixed).

Comment 9

[sairuh (rarely reading bugmail)]

also fails on WindowsXP. nominating for tbird 1.0.

forgot to note: username/password for the HTTP auth-protected feeds is:
testuser/testpass.

Comment 10

[Scott MacGregor]

taking

I think this is going to have the same fix as Bug #267665

Comment 11

[Scott MacGregor]

This issue and the issue with subscribing to a feed fails to bring up the http
proxy auth dialog are both regressions new in 0.9.

I think they are both caused by changes to nsDocShell as part of Bug #267367
where we were bringing up http auth prompts for favicon requests and other urls
running in chrome docshells. If I comment out those changes then this starts
working again. I don't have access to a proxy to test Bug #267665 to see if that
starts working again too but I suspect it will. 

cc'ing Darin as I should talk to him about possible ways to fix this for RSS.

Comment 12

[Scott MacGregor]

Attachment: work in progress...docshell work is incomplete

Comment 13

[Scott MacGregor]

Attachment: the fix

Comment 14

[Scott MacGregor]

Comment on attachment 166807 [details] [diff] [review]
the fix

This patch sets allowAuth to false for any docshell that isn't of item type
content (i.e. chrome). This allows us to get rid of the chrome check in
GetAuthPrompt and rely on mAllowAuth instead. 

That in turn allows us to modify the ::GetInterface version for nsIPrompt to
directly call GetAuthPrompt which is what Darin originally wanted to do in Bug
#267367

For the chrome docshell in mailnews (and the RSS subscription dialog), we now
explcility allow auth prompts because of the RSS case.

Note: we still disable auth prompts from the message pane so spam e-mails can't
bring up prompt notifications.

Comment 15

[Darin Fisher]

Comment on attachment 166807 [details] [diff] [review]
the fix

>Index: docshell/base/nsDocShell.cpp

> nsresult
> nsDocShell::GetAuthPrompt(PRUint32 aPromptReason, nsIAuthPrompt **aResult)
> {
>-    // if this docshell is of type chrome and has a chrome URI, then do not
>-    // give out an auth prompt.  NOTE: it is possible to load a non-chrome
>-    // URI into a chrome docshell, so this check is important.
>-    if (mCurrentURI && mItemType == typeChrome) {
>-        PRBool chrome;
>-        if (NS_SUCCEEDED(mCurrentURI->SchemeIs("chrome", &chrome)) && chrome)
>-            return NS_ERROR_NOT_AVAILABLE;
>-    }

We should check with jst, but I think that this is ok because 1)
we don't really care about loading non-chrome URLs into chrome
docshells, 2) other parts of the code only inspect the itemType,
and 3) if anything this is just more restrictive, not less.  So,
I think it's safe :)


>Index: mail/extensions/newsblog/content/subscriptions.js

>+  window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>+        .getInterface(Components.interfaces.nsIWebNavigation)
>+        .QueryInterface(Components.interfaces.nsIDocShell).allowAuth = true;

Isn't the last QueryInterface is extraneous?  That is, won't this
also work:

  window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
	.getInterface(Components.interfaces.nsIDocShell).allowAuth = true;

Comment 16

[Scott MacGregor]

this is checked into the aviary 1.0 branch so I can get testing help with this
in the short term.

Leaving open until the reviews are finished. 

Trix/Sarah, can you start trying this out in tomorrow's builds?

Comment 17

[Scott MacGregor]

Hi Johnny, can you chime in if you think our assumption is incorrect here?

from comment #15: 

> nsresult
> nsDocShell::GetAuthPrompt(PRUint32 aPromptReason, nsIAuthPrompt **aResult)
> {
>-    // if this docshell is of type chrome and has a chrome URI, then do not
>-    // give out an auth prompt.  NOTE: it is possible to load a non-chrome
>-    // URI into a chrome docshell, so this check is important.
>-    if (mCurrentURI && mItemType == typeChrome) {
>-        PRBool chrome;
>-        if (NS_SUCCEEDED(mCurrentURI->SchemeIs("chrome", &chrome)) && chrome)
>-            return NS_ERROR_NOT_AVAILABLE;
>-    }

We should check with jst, but I think that this is ok because 1)
we don't really care about loading non-chrome URLs into chrome
docshells, 2) other parts of the code only inspect the itemType,
and 3) if anything this is just more restrictive, not less.  So,
I think it's safe :)

Comment 18

[Darin Fisher]

Comment on attachment 166807 [details] [diff] [review]
the fix

sr=darin with the suggested change to subscriptions.js and provided jst is ok
with this patch.

Comment 19

[Johnny Stenback (:jst)]

Comment on attachment 166807 [details] [diff] [review]
the fix

Yeah, looks good to me. r=jst

Comment 20

[sairuh (rarely reading bugmail)]

tests from comment 8 work fine now --tested with 2004112311-0.9 on linux fc2.

Comment 21

[Scott MacGregor]

now fixed on the trunk too. Closing out. 

Comment 22

[trix supremo]

Looks good.  Authentication is now prompted, connection is established, and
messages are now transferred.

Comment 23

[trix supremo]

verified on all 20041123 builds

Comment 24

[Mike Kaply [:mkaply]]

This should go into 1.7.6