Bug 232982 - "Save password" checkbox behaves weird
Summary: "Save password" checkbox behaves weird
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: CVS (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4.1   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard: hasPatch
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-20 10:35 EDT by Szymon Brandys CLA
Modified: 2008-08-04 05:25 EDT (History)
1 user (show)

See Also:


Attachments
Fix v01 (4.85 KB, patch)
2008-07-30 09:04 EDT, Tomasz Zarna CLA
no flags Details | Diff
mylyn/context/zip (60.98 KB, application/octet-stream)
2008-07-30 09:04 EDT, Tomasz Zarna CLA
no flags Details
Fix v02 (5.10 KB, patch)
2008-08-04 05:21 EDT, Tomasz Zarna CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Szymon Brandys CLA 2008-05-20 10:35:54 EDT
Steps:
1) Go to CVS Repositories view
2) Open Properties dialog for any repo
3) Check "Save password" and press "Apply"
4) Close and reopen the dialog
5) The checkbox is not set
Comment 1 Tomasz Zarna CLA 2008-05-21 10:11:51 EDT
The same thing happens if you want to uncheck the option. It stays checked when the dialog is reopened.
Comment 2 Tomasz Zarna CLA 2008-05-23 06:27:51 EDT
 (In reply to comment #0)
> Steps:
> 1) Go to CVS Repositories view
> 2) Open Properties dialog for any repo
> 3) Check "Save password" and press "Apply"
> 4) Close and reopen the dialog
> 5) The checkbox is not set

If the checkbox is unchecked when you open the property page for the first time it indicates that the password isn't cached, we all know that. However, it can also mean that the pass is null even though there are some *****. Having null password will always result in the checkbox being cleared (see method CVSRepositoryLocation#getUserInfoCache which determines whether the checkbox should be checked or not). 

Szymon, can you confirm you have a valid password and the checkbox is still not set?

 (In reply to comment #1)
> The same thing happens if you want to uncheck the option. It stays checked when
> the dialog is reopened.

Investigating...
Comment 3 Tomasz Zarna CLA 2008-05-23 10:39:48 EDT
See also bug 229987.
Comment 4 Tomasz Zarna CLA 2008-07-28 10:37:46 EDT
After a quick look, I think the main issue here is that previously CVSRepositoryLocation.flushCache used Platform.flushAuthorizationInfo which "Removes the authorization information for the specified protection space and given authorization scheme." and now, with Secure Storage on the board the flushCache method calls ISecurePreferences.flush() which "Saves the tree of secure preferences to the persistent storage.". I guess replacing it with ISecurePreferences..removeNode() should make the check box work as it used to.

Adding Oleg on CC since I belive he's an expert in Secure Storage API.
Comment 5 Tomasz Zarna CLA 2008-07-30 09:04:32 EDT
Created attachment 108741 [details]
Fix v01

Patch that adjust the way CVS uses Secure Storage. Unchecking the "Save password" checkbox for a CVS repository, removes the node from Secure Storage tree, but the password can be still used during current session. Szymon, could you apply it and check if it makes any difference?
Comment 6 Tomasz Zarna CLA 2008-07-30 09:04:55 EDT
Created attachment 108742 [details]
mylyn/context/zip
Comment 7 Oleg Besedin CLA 2008-07-30 13:34:25 EDT
Looks good to me. One thing I'd change to be on the safe side, is to add "node.flush()" after "node.clear()" in the removeNode() method (and then remove "flushCache();" call in the dispose()).

This would save changes immediately rather then on Eclipse shutdown for the other calls to the removeNode(). 

Comment 8 Tomasz Zarna CLA 2008-08-04 05:21:05 EDT
Created attachment 109050 [details]
Fix v02

Thanks for your comment Oleg. This is updated version of the previous patch.
Comment 9 Tomasz Zarna CLA 2008-08-04 05:25:10 EDT
Released to HEAD and 3.4.x branch.