Ability to log out of HTTP Auth for individual site

NEW
Unassigned

Status

()

Core
Networking: HTTP
--
enhancement
13 years ago
9 months ago

People

(Reporter: dveditz, Unassigned)

Tracking

(Blocks: 4 bugs, {helpwanted})

Trunk
Future
helpwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-would-take])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
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.
(Reporter)

Updated

13 years ago
Blocks: 61681, 68409

Comment 1

13 years ago
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.
Keywords: helpwanted
Target Milestone: --- → Future
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

13 years ago
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

13 years ago
KISS.

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

Comment 5

13 years ago
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
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

13 years ago
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.
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

13 years ago
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

13 years ago
(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.
(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 mark@ky.net - 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 :)
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

13 years ago
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]*
(Reporter)

Updated

12 years ago
Depends on: 260186
Created attachment 195398 [details] [diff] [review]
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>.
Created attachment 195399 [details]
Screenshot of attachment 195389 [details]

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.
Attachment #195398 - Flags: review?(darin)
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

12 years ago
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!
Attachment #195398 - Flags: review?(darin) → review-
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.
Attachment #195398 - Attachment is obsolete: true
Created attachment 196174 [details] [diff] [review]
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 ...
Attachment #196174 - Flags: review?(darin)
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.
Blocks: 260186
No longer depends on: 260186

Comment 21

12 years ago
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!
Attachment #196174 - Flags: review?(darin) → review-
Created attachment 197248 [details] [diff] [review]
version 0.2

Updated to review comments.
Attachment #196174 - Attachment is obsolete: true
Attachment #197248 - Flags: review?(darin)

Comment 23

12 years ago
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.
Attachment #197248 - Flags: review?(darin) → review-
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?

Updated

11 years ago
Assignee: darin → nobody
Blocks: 287957
Blocks: 300002

Updated

8 years ago
Depends on: 521588

Comment 25

7 years ago
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
(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

6 years ago
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

6 years ago
(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

6 years ago
(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

4 years ago
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

2 years ago
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

2 years ago
Per-view logins are confusing, as conventional cookie-based logins are per-domain. Simultaneous logged-in and logged-out is possible using private browsing.
Whiteboard: [necko-would-take]

Updated

a year ago
Blocks: 1203030
You need to log in before you can comment on or make changes to this bug.