when closing mozilla you expect that you're logged of PSM too, at least I assumed. as my friend browsed to a site and my data was entered by mozilla I was surprised because he didn't have to enter my PSM password. 1) the try context menu needs a [log of psm] entry 2) auto-log-off should be configureable in the preferences, thus when all browser windows are closed you're logged off too

added tracking bug, changed to gui features

See also bug 86067, Closing all windows in -turbo mode keeps sessions informations.

PSM should be invisible to users, there should be no need to expose it. They wouldn't understand it anyway, since they don't 'login to PSM', and PSM is not part of the interface (AFAIK). In any case, there should be no need for them to take additional action beyond closing the last window.

psm is already visible to the end user, he has to type in a password, tasks-> P&S-> Password Manager he can log off, fine the default behaviour could be auto-log off, but users who don't want to log off will complain, thus configureable

This isn't PSM, this is Password Manager. At any rate, marking p3, major, and mozilla1.0, cc-ing morse

Well it's really a question of semantics as to whether we call this PM (password manager) or PSM (personal security manager). What's happening is that PM is using PSM. That is, PM calls on PSM whenever it needs to have some encryption or decryption performed, and it is up to PSM to determine whether the user is currently logged in and to request the master password if he is not. So it is PSM that has to get reset when the browser is closed down under quick launch. Now it's true that the tasks->privacy->password-manager->logout function in in the PM module. But all that the code in the PM module does is call a logout routine in the PSM module.

Created new bug 1087870 for the enhancements mentioned in this bug. Changing summary to reflect what I'm going to do which is "Log out of Password Manager/PSM after closing last open window"

Steve Morse, can you r=. Blake, if you're lurking, can you sr=. Thanks

Comment on attachment 56009 [details] [diff] [review] call wallet_expirepassword when last window closes r=morse

I think this bug just became a dup of bug 86067, "Closing all windows in -turbo mode keeps sessions information". If so, that's a problem, because both bugs are being actively worked on by different developers.

Why are you calling these things directly and spreading dependencies around when there is already a notification mechanism in place for this. See: http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/nsNativeAppSupportWin.cpp#2081 Not only that, wallet already happens to implement nsIObserver for the purpose of clearing itself on shutting down a profile (which is the same as what you want to do here)

Moving up to mozilla0.9.7

some questions about this: 1) Is the problem, as reported back on 09-23 still happening? That was right about the time the "session-logout" notification started being made. 2) If so, why isn't what's happening (http auth being cleared) enough to address this problem? The reason I ask is that, based on mail with morse when I was putting in support for "session-logout", morse said wallet had no concept of session so I deemed it enough to clear session passwords out of http (also mail but that's another bug) Even if it can be shown why it's needed to explicitly call wallet, it shouldn't be done that way and making whatever else needs to respond to a "session-logout" is the way to go. This is from a module dependency standpoint. Ask the modsquad.

wallet has no concept of session. That is correct. But PSM does have such a concept because it logs you in when you supply the master password. So it is psm that needs to be shut down. You are right about the modularity. So the logout from psm should not be done by using the routines in wallet but rather you should probably call the psm module directly (i.e., copy the code that is used in the wallet module to call the psm module.) Equivalently, wallet could call the psm logout when it is called upon to clear itself. That would be in the nsWalletlibService::Observe of nsWalletService.cpp. Just add a call to WALLET_ExpirePassword in that routine.

On second thought, relying on wallet to clear out the psm master password is wrong. What if there was no wallet module (it is in extensions recall). In that case the master password would never get cleared out. "What is the master password used for if not for wallet?" you ask. It is used for certs. So if the user has opened his cert database, he has unlocked his master pasword.

> What if there was no wallet module Good point. The PSM/NSS component implements nsIObserver to listen for profile changes. It could easily listen for "session-logout" and clear the password then.

Ok, just posted new patch. I assume that I'll need someone with security checkin privilages to get this into the tree. ;-) I was thinking about either changing the name of nsNSSComponent::RegisterProfileChangeObserver() or creating another function to register 'session-logout' but I got lazy and didn't. ;-)

cc'ing kai because he knows this stuff - and is making huge changes to this code at the moment.

Fixes for various bugs are in the upcoming patch in bug 75947. In the near future, when we see profile-before-change, NSS will be completely shut down. Is that enough? Or should we still log security out on session-logout?

When the user closes the last window in turbo mode, only a session-logout is sent, not a profile shutdown. This is because the profile shutdown/startup was too expensive when the user picked the same profile each time (usual case). On a session-logout, observers just flush passwords and logout of things, which is cheaper. Can you see comment #18 and #19 and make sure PK11_LogoutAll does what we want here?

I understand. I asumme, when you start the application again, and it should happen that this is done with a different profile, you will still send both profile-before-change and profile-after-change. Yes, I think function PK11_LogoutAll is correct. Please hold this patch until we haved closed bug 75947, which will be hopefully tomorrow. Then we should add a safety check to your patch, to make sure NSS is currently in initialized state, and only if it is, execute PK11_LogoutAll.

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

Comment on attachment 59944 [details] [diff] [review] also check for mNSSInitialized before calling PK11_LogoutAll() in Observe() Your patch will not apply due to conflicts, as the context changed completely. But besides from that your suggested change is fine. r=kaie

Comment on attachment 60296 [details] [diff] [review] update patch to today's trunk kaie already gave r=

cc-ing alecf for sr

Comment on attachment 60296 [details] [diff] [review] update patch to today's trunk sr=alecf

fixed checked in to trunk