144256 - giving invalid username or password after first successful publish still publishes!

Status: RESOLVED WONTFIX

(Core :: Networking, defect, P2)

Description

[sujay]

using 5/13 trunk and branch builds

1) launch netscape
2) launch composer
3) enter some text
4) use http publish to publish this page somewhere; make sure you enter a valid
username and passwd.

so page has succesfully published.

5) make some more edits to that page
6) File | Publish As
7) enter an invalid username and/or passwd.
8) Publish

it still publishes!

expected: the enterprise server passwd dialog should come up prompting
for the correct username and passwd.

Comment 1

[sujay]

nominating...

Comment 2

[sujay]

changing component...

Comment 3

[Darin Fisher]

mass futuring of untargeted bugs

Comment 4

[benc]

-> back to editor.

This might be password manager, but I HTTP:auth has worked pretty well as of
late, to the best of my knowledge.

Comment 5

[Syd Logan]

Kathy, can you comment on this? Sounds like we prompt for the user name and
password but because the file was already successfully published, "save as" is
really behaving like "save", using what is already known about the location and
authentication, and ignoring what was entered. 

Comment 6

[Kathleen :Brade]

Sujay--can you still reproduce this problem?  Does it publish with the old file
name and to the old location?

Comment 7

[Charles Manske]

I can still reproduce this problem. I can't think of what we can do on the
Composer side. After publishing successfully via HTTP, I can create a new file,
do Publish (brings up dialog just like Publish As) and change the filename and
username (to something not legal) and it still publishes "successfully". 
Until you close the app, any publishing to the the same server seems to use the
original authorization.
I don't think this has anything to do with Password Manager. I tested with the 
"Save password" unchecked and an empty password; after it prompted me for
username and password (and Composer didn't save the password), it still publishes
after the filename and username are changed using Publish As dialog.

Comment 8

[benc]

There are some necko/http level authentication caches. Darin could probably
explain better than I...

Comment 9

[Darin Fisher]

HTTP automatically remembers "successful" logins.  short of clearing all logins,
i can't think of a way to avoid this problem w/o introducing some sort of new
API to necko.  perhaps all that is needed is a load flag which tells HTTP to not
use its auth cache?!

Comment 10

[Charles Manske]

A new load flag sounds good to me. We would set it ONLY if the username and/or
password change for a particular server.
I assume the auth-caching is per server?

Comment 11

[Darin Fisher]

to bad nsIHttpChannel froze or we could have added an attribute there.  a load
flag would work (as i suggested), but i hesitate a bit because load flags are
meant to be generic across different channels.  maybe we could generalize "allow
using cached auth" or something like that.

Comment 12

[Bradley Baetz (:bbaetz)]

Can't we change http to not use the cached data if you're given user:pw@host
form or url? Thats what composer uses, isn't it?

Comment 13

[Darin Fisher]

bbaetz: well, the problem is that composer is specifying an invalid user:pw in
the URL, so when HTTP detects an invalid user:pw, it looks in its auth cache to
see if there is a valid user:pw for the auth domain.  if so, then it uses it. 
otherwise, it prompts the user.  i don't think we can change this behavior in
general without breaking things (e.g., you should be able to author content with
a bogus user:pw in the URL and then only have the user prompted once).

Comment 14

[Bradley Baetz (:bbaetz)]

If I type the url 'http://invalid:[e-mail]/', then I expect to get an error
message. Anything else is incorrect.

Can't we change the url spec, and have composer ask for the 'real' url somehow,
which it can store for later use?

Comment 15

[Charles Manske]

Bradley's suggestion is very good, but the fact that we use the embedded 
"username:password@" format sucks anyway from the security point of view :(

Comment 16

[Darin Fisher]

consider this example (http://blah.com/foo.html):

<html>
<body>
<img src="http://wrong:[e-mail]/image1.png">
<img src="http://wrong:[e-mail]/image2.png">
</body>
</html>

should the user see two broken image icons, or should the user be prompted
_once_ for the correct username and password?

Comment 17

[Charles Manske]

The way publishing works now, the embeded username:password of any image is always
the same as for the HTML page, so the user should be prompted once and all
instances of the wrong data should be corrected.

Comment 18

[Darin Fisher]

...and the way that is currently supported is by necko looking in its HTTP auth
cache when the username:password specified in the URL results in a 401 failure
code.  hence, comment #13 ;-)

Comment 19

[Charles Manske]

Darin or Bradley: If we don't set the username and password in the URL for 
images in URI object for associated files (the "aData" param in 
nsIWebBrowserPersist::saveDocument), will it try to use authentication obtained
for the html file? 

Comment 20

[Darin Fisher]

yes, provided the images have the same hostname as the toplevel HTML document,
which is what i assume you meant.

Comment 21

[Charles Manske]

Attachment: Composer part of fix v1

I tested my suggestion in comment #19 and it does seem to work fine. 
I think NOT setting the username and password on the "other files" URI object
will
simplify fixing this issue.

Comment 22

[Kathleen :Brade]

I think the fix is in http code, not in Composer.

In comment 12, Bradley says:
>Can't we change http to not use the cached data if you're given user:pw@host
>form or url? Thats what composer uses, isn't it?

In comment 13, Darin says:
>bbaetz: well, the problem is that composer is specifying an invalid user:pw in
>the URL, so when HTTP detects an invalid user:pw, it looks in its auth cache to
>see if there is a valid user:pw for the auth domain.  if so, then it uses it. 
>otherwise, it prompts the user.  i don't think we can change this behavior in
>general without breaking things (e.g., you should be able to author content 
>with a bogus user:pw in the URL and then only have the user prompted once).

If the user doesn't match the cached data, the cached data shouldn't be used. 
Specifying a login in an url should cause this check to happen and only use
cached data if it synchs.  I'm not sure I agree with Darin's example for only
prompting once.  For the example in comment 16, I think the user should see two
broken images.

The image issue is an edge case for this bug.  The example / steps in the
original comment don't mention any images in the document at all.

If I am publishing to locations, I think it is valid to publish first to
  http://[e-mail]/index.html
and then to publish as to this location:
  http://[e-mail]/index.html
and then a third time to this location:
  http://[e-mail]/index.html

Comment 23

[Darin Fisher]

hmm... well, perhaps we simply need two different policies: one for publishing
and one for page load.  that seems to be where load flags come into play. 
either way, i think the solution is definitely going to require changes to necko.

here's an internal testcase with some inline images:

http://unagi/bugs/bug_144256/test.html

incidentally, it seems NN 4.79 shows broken image icons for all of these
presumably because it does not support HTTP authentication for inline images,
which is very surprising to me.

Comment 24

[Kathleen :Brade]

Darin--your example (http://unagi/bugs/bug_144256/test.html) looks fine; what do
you intend to happen when such a page is loaded?  Which cases should pose the
prompt?  All of them?  None of them?  Some of them?

Can I reassign this bug to someone else?  I'm not sure I'm the right person to
fix it (unless everyone else is overloaded).

Comment 25

[Kathleen :Brade]

Comment on attachment 108813 [details] [diff] [review]
Composer part of fix v1

I don't think we want this fix

Comment 26

[Kathleen :Brade]

-->darin for further triage/better fix

Comment 27

[Kathleen :Brade]

Darin--is this a duplicate of bug 143575 or bug 140645?

Comment 28

[Darin Fisher]

yeah, they are probably related... not sure if they are exactly duplicates though.

Comment 29

[Ken Johanson]

I think I just saw the opposite effect of this bug. I:

1) Created a new publish site, and entered INVALID credentials.
2) Checked the "save password" box.
3) Tried to publish (failed per bad creadentials).
4) Entered VALID credentials in the same "site" dialog.
5) Published and still got a failed message (server log still showed old
credentials being sent)

To get it to work, I had to create a NEW site and enter the correct credentials.

PS -just upgraded from Moz1.2 to 1.3b... Mozilla R-O-C-K-S!!! :)

Comment 30

[Samir Gehani]

adt: nsbeta1+/adt2

Comment 31

[Paul Wyskoczka]

ADT: Nominating topembed

Comment 32

[Michael Buckland]

Discussed in top embed bug triage.  Plussing.