> On April 14, 2011, 12:12 a.m., gustavs wrote:
> > From manual testing
> >
> > 1) a XMPP account I enter(ed), is not loaded on the next start. Saving/restoring methods are called, but loading values are empty. I do miss the "enter storage passphrases" dialog I remember from previous revision. I didnt noticed Exceptions.
> >
> > 2) when enabling/disabling password encryption, I think the account will not be stored with the corresponding mode. The changed encryption preference is not yet stored in the prefstore when saving accounts happens - so the old value of encryption setting is used when deciding to save encrypted or not.
> >
> >
> >
>
> Karl Beecher wrote:
> 1) The code for converting accounts (from normal preference store to secure store) should only be run after a version update or if you enable assertions (although I remember Björn K. pointed out a problem with StartupSaros.java:104 - I think the logic is slightly wrong. It should be "!assertedEnabled" so that it continues. Amend your copy of the patch this way to test it).
>
> Also, I removed the preference initialisers (they were overwriting the active account data). The secure store doesn't need them, because you can specify default values in case the preference key doesn't exist.
>
> 2) It seems to work for me. You can verify by turning on encryption, clicking OK, then going back to preferences (General -> Security -> Secure Storage) and looking in Contents tab -- if encryption happened, all values should be "*******". Let me know if this doesn't happen for you.
>
> gustavs wrote:
> 1) Ah, ok
>
> 2) I dare to say, it doesnt happen for me. As i commented the code, when changing the encryption setting, the new value is not used for saving accounts but the previouse encryption setting as stored in the prefstore. The second time saving the current encryption setting is used. The way I tested: When accounts are not encrypted and the checkbox is unchecked -> check the checkbox -> press apply --> check secure storage -> notice its not encrypted -> press apply again -> --> check secure storage -> notice its encrypted
>
> When debugging i notice in the line I commented on that the encrypt flag taken from the prefstore is not yet updated when I just changed the checkbox state. Its like the checkbox state is stored in the prefstore after saving accounts which is taking the encrypt flag from prefstore and not from the GUI.
>
> I hope this makes sense
> (this is also based on the current revision 7)
Hmm, this task continues to task me.
Thanks for the feedback, I'll take it into account!
- Karl
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://saros-build.imp.fu-berlin.de/reviews/r/26/#review90
-----------------------------------------------------------
On April 14, 2011, 11:52 a.m., Karl Beecher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://saros-build.imp.fu-berlin.de/reviews/r/26/
> -----------------------------------------------------------
>
> (Updated April 14, 2011, 11:52 a.m.)
>
>
> Review request for Saros.
>
>
> Summary
> -------
>
> Thanks to those who reminded me about the XMPP password encryption issue. This has been owned by a few people in the past, but with no result, so I've taken it up myself today.
>
> I basically thought of two approaches:
>
> 1) Using the ISecurePreferences, Eclipse's standard way of storing encrypted preference data (available through org.eclipse.equinox.secure).
>
> or 2) Using Java's own encryption methods to encrypt/decrypt the password text when storing/retrieving the password.
>
> I went for option 1, mostly because it's the simple "Eclipse Way" of doing things, but also because option 2 is a bit messy and requires fiddling with actual encryption details (algorithms, keys, etc).
>
> Warning: the XMPPAccountStore is a bit flaky and causes problems, so I would like to improve it later.
>
>
> This addresses bug 3065127.
> http://sourceforge.net/support/tracker.php?aid=3065127
>
>
> Diffs
> -----
>
> /trunk/dpp/de.fu_berlin.inf.dpp/META-INF/MANIFEST.MF 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/Saros.java 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/StartupSaros.java 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/accountManagement/XMPPAccountStore.java 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/preferences/PreferenceConstants.java 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/preferences/PreferenceInitializer.java 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/preferences/PreferenceUtils.java 3262
> /trunk/dpp/de.fu_berlin.inf.dpp/src/de/fu_berlin/inf/dpp/ui/preferencePages/GeneralPreferencePage.java 3262
>
> Diff: http://saros-build.imp.fu-berlin.de/reviews/r/26/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karl
>
>
|