320208 - Reset Camino clears out Keychain entries from other apps

Status: VERIFIED FIXED

(Camino Graveyard :: OS Integration, defect)

Description

[Chris Lawson (gone)]

Reset Camino (resetBrowser in MainController.mm) calls removeAllUsernamesAndPasswords on an instance of KeychainService. The code for that method appears to step through the user's entire keychain, deleting *all* HTTP entries, whether or not Camino put them there.

This is a Bad Thing(tm) for anyone who uses multiple browsers that have the capability of storing site passwords in the Keychain, most notably Safari.

Comment 1

[Chris Lawson (gone)]

Asking for blocking 1.0. This is pretty bad.

cl

Comment 2

[Lee Henderson]

Sorry, with Carbon Keychain Manager API, there is no way to know which Application created a keychain entry.

There is a bug which would move us to the new Keychain Services API (MacOSX10.2+), which the Kind field CAN tell you who wrote it, if you choose to create a kind for Camino Password/Camino Web form, instead of sharing those kinds with Safari.

I submit that it results in poor user experience not to share that database with Safari.

I would suggest the Reset Camino feature should not delete any keychain data without a specific option presented to the user that should alert the user that other saved web browser form and password data will be eliminated.

Comment 3

[Håkan Waara]

We were discussing this on IRC, and perhaps it's better to not touch the user's passwords until we can remove those that we 100% know are created by us?

That is, turn off removing of keychain passwords until bug 172842 is fixed and we have a new keychain system.

Comment 4

[Lee Henderson]

our options to alter behavior of reset camino:
1. do not delete the keychain passwords anymore
2. present user with a checkbox "Also Delete all Internet Keychain Passwords and Web Form Data" in reset camino dialog to clear them, defaulted off

since the work has already been done to reset the keychain passwords, i think its a better solution to add the checkbox in the dialog

some users will want to do it, for whatever reason

Comment 5

[Håkan Waara]

I don't think it's feasible to remove the passwords in this manner since we may remove passwords that other applications (Safari) have created.

The feature is "Reset Camino", not "Reset Camino and part of Safari". :-) 

That's why I'd suggest not removing any passwords, until we have a working keychain implementation in place to deal with this properly.

Comment 6

[Håkan Waara]

Attachment: Proposed patch

This patch simply comments-out the removing of passwords - and references this bug for the future implementor of the new Keychain Services API.
It also changes the wording of the "Reset Camino" dialog to reflect the fact that we not touch usernames/passwords.

Comment 7

[Simon Fraser [no longer active]]

I put an #if 0 around the -removeAllUsernamesAndPasswords call for now.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I didn't see a localizable.strings change with this in bonsai, and at the very least I think we need to remove the existing "and remove all remembered usernames and passwords" bit from the "Reset Camino" message.  

Even better would be a note along the lines of Håkan's proposed change, indicating that usernames and passwords saved by Camino in the Keychain will not be removed.

Comment 9

[Simon Fraser [no longer active]]

I'll fix the string to something reasonable. Maybe some newlines can highlight the fact that keychain entries will not get removed.

Comment 10

[Simon Fraser [no longer active]]

Fix the warning string along with checkins for bug 321882.