260839 - Ability to log out of HTTP Auth for individual site

Status: NEW

(Core :: Networking: HTTP, enhancement, P5)

Description

[Daniel Veditz [:dveditz]]

Bug 55181 added the ability to clear all HTTP Basic Auth login info without
having to shut the program down entirely, logging out of all sites.

A much requested refinement is the ability to log out of only the current site
without having to log out of other sites you may be connected to. Split from bug
55181, RIP.

Comment 1

[Darin Fisher]

We'd need to add a method (or methods) to nsIHttpAuthManager to support this. 
It may be enough to simply have a method that clears any credentials for the
given URL.  But, what about proxy authentication?  That should probably be
handled separately.

Comment 2

[Vidar Haarr (not reading bugmail)]

I did some research myself around the interfaces when playing with bug 55181,
and came to the same conclusion as darin -- of course, since he has written the
entire thing, we already know he's right :)

I was thinking about some other issues as well - the UI. I say just make it
easy, and add another menuitem in the Tools->Password Manager submenu: "Log out
<URL>". At the same time, the "Log out" should be renamed "Log out
mastersession" or similar, and the menuitems should be greyed out when not
applicable.

Everything above should be possible with the current API, AFAIK - we only need a
way to specifically log out from one session.
Darin: can we use nsIHttpAuthManager.getAuthIdentity to check if the current tab
has a logged in session?

Comment 3

[Sander]

Something to keep in mind: I doubt most users have any idea of "HTTP Basic
Auth".  If we go and do site specific logging out, it might become worthwhile
overloading the ui entry point to also clear all session (?) cookies from the
same url, for all the systems which use those for logging in. (Of course that
isn't perfect, as some sites will use persistent cookies - but then, the harm in
deleting the wrong persistent cookies is larger, so I think deleting session
cookies would be the least-wrong option here.)

Comment 4

[David Balažic]

KISS.

There is a HTTP login, add a HTTP logout and be done with it.

Comment 5

[Benoit]

I like the suggestion made here, using a login/logout bar at the top of the
window (see the full thread for more information on how it could work) :
http://listserver.dreamhost.com/pipermail/whatwg-whatwg.org/2004-December/002752.html

Comment 6

[Vidar Haarr (not reading bugmail)]

Interesting idea, but I think it needs more refinement. First of all, having
that bar displaying with "Log out" *all the time* when logged in, will reduce my
already overloaded workspace even further.

Hiding it won't be difficult; we'll just provide a close-button (X) in the top
right corner, just as the other "information bars."

Showing it again will be the difficult part. I've pondered about it, but I can't
seem to find any good ways to make it reappear (statusbar icon, menuitem,
tab-thingy, urlbar-thingy, they're all bad).

Comment 7

[Martin Bähr]

indicating that you are logged in could be done the same way as https is
indicated currently. since both relate to security somehow.
like a lock showing that https is used, a key could show that a password is
used. clicking on the key will bring up the logout dialog.

Comment 8

[Vidar Haarr (not reading bugmail)]

Yes, indeed, but where? Having a statusbaricon is not enough (discoverability).
There's no place in the urlbar if the site is secure. We can't have it on the
tabs, because then everybody in the world would turn I'm-the-UI-expert on you. I
don't want to add a menuitem, because there are way too many already.

Oh, and from mpt's design, I think the "Stay logged in"-option should be
removed. Also, does anyone know where he gets the "Committee Members Area"-text
from?

Comment 9

[Benoit]

I assume it's the name of the restricted zone (AuthName) defined in the
.htaccess file.

See http://httpd.apache.org/docs/howto/htaccess.html#auth ; with this file
applied it would say "Password Required" instead (which IMHO is not a very good
name to use, but it's just an example)

What we have now is a popup window saying "Enter your password for: 'Password
Required' on some.website.com".

Comment 10

[Mark Bitterling]

(In reply to comment #8)
> Yes, indeed, but where? Having a statusbaricon is not enough (discoverability).

Whether or not it provides enough discoverability, the status bar is the most logical place to put it.  

Why not implement it in the status bar and then we can address improving discoverability later if 
necessary.

Comment 11

[Vidar Haarr (not reading bugmail)]

(In reply to comment #9)
> I assume it's the name of the restricted zone (AuthName) defined in the
> .htaccess file.

Yes, thanks for the pointer.

I agree with [e-mail] - we should just add it to the statusbar, and some GUI
guru can jump us and force a rewrite afterwards.

I'm guessing we need to use the browsermessage stuff, and create a new prototype
like "credentialsNeeded" or something in
<http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js> (there
is lots of code there already that can be reused/copied).

In any case, it doesn't seem like browsermessage
(<http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/browser.xml#871>)
support more than a predefined set of elements (text, button, close, image), so
it would have to be extended or duplicated to let it contain other stuff. I'm
tempted to say that it should be just another frame that could #include a XUL
file, but then we'd have to rewrite the current stuff as well. Thoughts?

Regarding the prompt, I have absolutely no idea how we could change it. I think
embeddors can do it, however, so we should be able to. I'm guessing
<http://lxr.mozilla.org/seamonkey/source/embedding/components/windowwatcher/src/nsPromptService.cpp>
is the class that provides the actual dialogs, so perhaps we can create a new
implementation that override ::PromptUsernameAndPassword and/or ::PromptPassword ?

I'd love to get some comments and suggestions from darin or biesi!
CC'ing biesi for feedback :)

Comment 12

[Vidar Haarr (not reading bugmail)]

It just occured to me that this bug is not the right place to discuss
redesigning our authprompt/implementing a "log in" and "who am I logged in as"-bar.

I think the scope of this bug should be to add a statusbar icon for sites where
you are logged in, with an the contextmenu item "Log out from <site>" (which is
what the initial description is about).

Please comment in bug 276412. Sorry about the spam :/

Comment 13

[[e-mail]]

You know of course that status bar icons aren't keyboard accessible, so that
you'll also need Tools > Password Manager > Log Out of Site > [Auth Realm]*

Comment 14

[Vidar Haarr (not reading bugmail)]

Attachment: Browsermessage prototype

Alright .. This is a first stab/prototype of using a browsermessage for showing
currently logged in state and a Logout button for Firefox - for Seamonkey
I/someone will probably do what Neil suggested in comment 13.

The patch does alot of things that sorely need review:
 * Adds a new out AString aUserRealm to nsIHttpAuthManager->getAuthIdentity,
because I wasn't sure how to get the Realm, and I knew it was available there.
I'm sure Darin has another way of accessing it :-)
 * Adds a new method clearAuthEntry(...) to nsIHttpAuthManager for clearing the
auth cache for a single site. Maybe we should support clearing by path/realm as
well?
 * Currently only supports logins in HTTP, not HTTPS. nsIURI.port returns -1 if
it is the default port for the current scheme, and I have no idea how to get
the actual port .. but I'm sure there is some property for it ?
 * I just made it into a browsermessage because I think it looks cool :-) I'm
not at all sure that bryner/mconnor/someone wants it like this at all.

I'm not sure who to request review from, but I'd like several people to take a
look at this patch - maybe darin, mconnor? Maybe it should be divided in two as
well, but I'm just attaching it all here so that we can get an idea of what the
final result will be.

I'll attach a screenshot shortly.

You can test the patch at <http://gemal.dk/browserspy/password.html>.

Comment 15

[Vidar Haarr (not reading bugmail)]

Attachment: Screenshot of attachment 195389

I noticed that in the patch, the access key for Logout is "L", but in the
screenshot it's "o". The patch is the most recent one.

Comment 16

[Vidar Haarr (not reading bugmail)]

Comment on attachment 195398 [details] [diff] [review]
Browsermessage prototype

Please note that I did not patch the only existing consumer of
nsIHttpAuth->getAuthIdentity;
<http://lxr.mozilla.org/mozilla/source/modules/oji/src/nsJVMAuthTools.cpp#162>,
because I compiled with --disable-oji. If you'd like it patched in this
version, let me know - personally I think it can wait untill we know if this
API change is wanted or not.

Comment 17

[Darin Fisher]

Comment on attachment 195398 [details] [diff] [review]
Browsermessage prototype

Can you please put the C++ changes and the UI changes into separate patches? 
Different people will need to sign-off on the different parts of the codebase
that this patch touches, and separate patches will make that easier to do. 
Thanks!

Comment 18

[Vidar Haarr (not reading bugmail)]

Comment on attachment 195398 [details] [diff] [review]
Browsermessage prototype

Yes, I can attach the C++ changes in a seperate patch. I wanted to attach it as
a whole in hope that it would be easier to review the API changes if you could
see a consumer of them at the same time ...

mconnor said that he did not think much of using a browsermessage for this at
first sight, but that Login/Logout in general could possibly be one of the
focus areas for Firefox 2.0. With that in mind, I'll obsolete this patch and
attach a new UI patch to a seperate bug once the API changes are in.

Comment 19

[Vidar Haarr (not reading bugmail)]

Attachment: version 0.1

* Adds a new out AString aUserRealm to nsIHttpAuthManager->getAuthIdentity,
because I wasn't sure how to get the Realm as a consumer, and I knew it was
available there.
I'm sure Darin has a better way of accessing it :-)
 * Adds a new method clearAuthEntry(...) to nsIHttpAuthManager for clearing the

auth cache for a single site. Maybe we should support clearing by path/realm as

well? Do you think ClearAuthEntry should return an error code if it fails? It
just returns NS_OK unconditionally in this patch.
 * The way the API is now, a consumer must try to get the Auth entries for a
site and rely on return errors to determine if there is one or not. Maybe a
HasAuthIdentity method could be added. Alternatively, it might be an option to
add a nsIObserver type "login-succeeded" (or similar) that interested consumers
could listen to ...

Comment 20

[Vidar Haarr (not reading bugmail)]

dveditz: I think it makes more sense that the GUI implementation bug depends on
the API implementation, and not the other way around, so I'm switching bug
260186 from depends to blocks.

Comment 21

[Darin Fisher]

Comment on attachment 196174 [details] [diff] [review]
version 0.1

>Index: netwerk/protocol/http/public/nsIHttpAuthManager.idl

>     void getAuthIdentity(in ACString aScheme,
...
>-                         out AString aUserPassword);
>+                         out AString aUserPassword,
>+                         out AString aUserRealm);

Whenever you change an interface definition, you must change
the interface's uuid property.


>Index: netwerk/protocol/http/src/nsHttpAuthManager.cpp

>+  aUserRealm.Assign(NS_ConvertUTF8toUCS2(entry->Realm()));

This code involved an intermediate buffer that can be avoided
like so:

    CopyUTF8toUTF16(entry->Realm(), aUserRealm);


Please post a revised patch.  Thanks!

Comment 22

[Vidar Haarr (not reading bugmail)]

Attachment: version 0.2

Updated to review comments.

Comment 23

[Darin Fisher]

Comment on attachment 197248 [details] [diff] [review]
version 0.2

>Index: netwerk/protocol/http/public/nsIHttpAuthManager.idl

>     void getAuthIdentity(in ACString aScheme,
>                          in ACString aHost,
>                          in PRInt32  aPort,
>                          in ACString aAuthType,
>                          in ACString aRealm,
>                          in ACString aPath,
>                          out AString aUserDomain,
>                          out AString aUserName,
>+                         out AString aUserPassword,
>+                         out AString aUserRealm);

OK, when I first reviewed this patch, it did not occur to me
that aUserRealm is basically the same thing as aRealm, except
that it is an out-param instead of an in-param.  Moreover,
aRealm is an optional in-param.  So, if you are going to add
an out-param for aRealm, then why not do so for aAuthType and
aPath as well?	Also, why shouldn't the type of aRealm and
aUserRealm be the same?  Why is one ACString while the other
is AString?  That seems inconsistent and wrong.  I notice that
you seem to assume that aRealm's value is UTF-8 encoded, but
how do you know that?  In fact, I don't think it is.

I think a better solution here would be to declare aAuthType,
aRealm, and aPath all to be inout params.  Then, make it so
that those values may be optionally non-empty when the function
is called, but they will be filled in upon return if they are
empty.

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

while we are touching this interface... should aHost become an AUTF8String? Most
of necko does that, for IDN. what does this interface do for IDN?

Comment 25

[David Balažic]

Thawing sequence initiated... 3 ... 2 ... 1 ...... Welcome to year 2010.

With Firefox 4.0 about to be released, it might be a good opportunity to revisit this bug.

And an idea about discoverability :

On first HTTP login, pop up* a dialog with an explanation and a "do not show this in future" option, like with form history, non-secure submission etc.

* - or just use the login dialog for this purpose

Comment 26

[Henri Sivonen (:hsivonen)]

(In reply to comment #25)
> On first HTTP login, pop up* a dialog with an explanation and a "do not show
> this in future" option, like with form history, non-secure submission etc.
> 
> * - or just use the login dialog for this purpose

IIRC, Account Manager (not in Firefox 4) has UI for this. In any case, using the Account Manager UI makes more sense than dialogs.

Comment 27

[singpolyma]

Bug 55181 seems to have introduced this sort of UI for non-Firefox back in the day.  Is this bug just something no one is interested in working on and that's why it hasn't been done yet?  This sort of UI really should always have been standard.  Not being able to log out without closing the browser is a fairly big deal.

Comment 28

[Jo Hermans]

(In reply to singpolyma from comment #27)
They're working on this (successor to Account Manager, mentioned above):
<http://identity.mozilla.com/post/8841090082/sign-into-websites-directly-from-your-browser-toolbar>

which is much more than a simple login/logout. But it's something totally different of course.

> Bug 55181 seems to have introduced this sort of UI for non-Firefox back in
> the day.  Is this bug just something no one is interested in working on and

That was using the Password Manager, which doesn't exist anymore (not in that form at least) in Firefox.

> that's why it hasn't been done yet?  This sort of UI really should always
> have been standard.  Not being able to log out without closing the browser
> is a fairly big deal.

Since you have the problem in practically all browsers, most websites put a logout button somewhere on their page. Which will also deal with the session-cookies, etc ...

Comment 29

[singpolyma]

(In reply to Jo Hermans from comment #28)
> (In reply to singpolyma from comment #27)
> They're working on this (successor to Account Manager, mentioned above):
> <http://identity.mozilla.com/post/8841090082/sign-into-websites-directly-
> from-your-browser-toolbar>
> 
> which is much more than a simple login/logout. But it's something totally
> different of course.

It also doesn't seem to actually provide a "logout" button on HTTP BasicAuth pages.  At least not on my test page at http://singpolyma.net/t/ (username singpolyma, password test)

> > Bug 55181 seems to have introduced this sort of UI for non-Firefox back in
> > the day.  Is this bug just something no one is interested in working on and
> 
> That was using the Password Manager, which doesn't exist anymore (not in
> that form at least) in Firefox.

Right, makes sense.

> > that's why it hasn't been done yet?  This sort of UI really should always
> > have been standard.  Not being able to log out without closing the browser
> > is a fairly big deal.
> 
> Since you have the problem in practically all browsers, most websites put a
> logout button somewhere on their page. Which will also deal with the
> session-cookies, etc ...

The problem is that the hack solution that you mention is to send a 401 to the UA even though the credentials are valid.  The only thing the UA knows how to do at that point is ask for credentials again.  This is a terrible user experience.  When a user clicks "log out" they don't expect to be asked to log in.  They expect to be logged out.

Comment 30

[Tim Stewart]

This is a feature I would really appreciate.  Personally, I believe HTTP auth would be more popular if browsers had a proper user experience when using it.

I have the ability to work on this bug, but do not have Mozilla-specific experience.  Is there a particular reason it has been idle so long?  Is it already covered by some other medium- to long-term plan?

Comment 31

[Mildred Ki'Lya]

I am also interested in this bug, and I think it is two part:

- first, make sure that the core objects supports logging out of a single website. It would be best if we could log out on a per-view basis and not per-domain. I might want to be logged-in to example.com in one window and be logged-out of example.com in another window.

- then, provide UI for logging out, or let an addon do that separately.

The patch attached to this bug is changing the .idl interface to allow logging out of a specific site using a scheme-host-port combination to identify the credentials in the cache. This covers the first part of the problem. It seems that there were a few remarks on the code and this was never acted upon.

Comment 32

[Gábor Stefanik]

Per-view logins are confusing, as conventional cookie-based logins are per-domain. Simultaneous logged-in and logged-out is possible using private browsing.

Comment 33

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 34

[jeti2121]

The browser asks to save the password, by showing a key-icon in the address bar. Why can this key-icon not stay there, while using Basic Authentication. And then provide the option in a menu there to logout or login as another user for this particular site? And for the accounts that were saved in the browser profile, it should show a list of accounts to login with just a simple click. This also makes it visible that you are logged in using Basic Authentication, which is currently not the case.

The adoption of Basic Authentication is crippled due to this incomplete implementation. Basic Authentication is very useful. Of course many websites use regular login forms in combination with cookies, but that does not mean Basic Authentication is dead and cannot be revived. The RFC 7235, section 6.2 states:

User agents that cache credentials are encouraged to provide a
readily accessible mechanism for discarding cached credentials under
user control.

Letting the browser take care of authentication in a uniform method would have been very desired, just like it offers to remember login details and can generate random passwords. And I would argue it would add another layer of security as well, since rogue javascript code (XSS) can easily intercept a <input type="password"> field, but this is not true for Basic Authentication. Just like javascript is not allowed to touch httpOnly session cookies. This actually makes Basic Authentication - given a long random password is used - even more secure as a permanent session cookie. Furthermore it enhances privacy, since the login information is not stored in cookies, of which the browser cannot know which one is used for logging in. Clearing cookies on a website currently deletes everything, so separating authentication from cookies would make sense.

Given Mozilla's mission, it's surprising that this issue is still open after almost two decades.

Comment 35

[David Balažic]

Is there any good reason to not do the clearing of HTTP Auth in the "Clear cookies and site data…" menu/dialog?

Comment 37

[Randell Jesup [:jesup] (needinfo me)]

Before it makes sense to implement this backend ability, we need to have a design and commitment to implement said design (otherwise it's just a latent ability that users can't access). This means a decision in bug 708132 before we can go forward with this.