Community
Participate
Working Groups
We now have support for use of the secure storage in the java class TaskRepository.
Created attachment 167670 [details] Migration process support
Added a patch that adds migration support for any interested connector. All you need to do is call MigratoToSecureStorageJob with right connector kind. Also enabled Secure Storage for Bugzilla. Looking forward for your feedback.
Created attachment 167937 [details] Migration updated There was a bug in the previous version.
After updating to the latest version of the JIRA connector I noticed that I was getting prompted for the password to the secure storage before the workbench was launched: "main" prio=10 tid=0x08c76800 nid=0x3525 runnable [0xbfdcc000] java.lang.Thread.State: RUNNABLE at org.eclipse.swt.internal.gtk.OS.Call(Native Method) at org.eclipse.swt.widgets.Display.sleep(Display.java:4019) at org.eclipse.jface.window.Window.runEventLoop(Window.java:826) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.equinox.internal.security.ui.storage.DefaultPasswordProvider$1.run(DefaultPasswordProvider.java:49) at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:179) at org.eclipse.ui.internal.UISynchronizer.syncExec(UISynchronizer.java:150) at org.eclipse.swt.widgets.Display.syncExec(Display.java:4280) at org.eclipse.equinox.internal.security.ui.storage.DefaultPasswordProvider.getPassword(DefaultPasswordProvider.java:47) at org.eclipse.equinox.internal.security.storage.PasswordProviderModuleExt.getPassword(PasswordProviderModuleExt.java:35) at org.eclipse.equinox.internal.security.storage.SecurePreferencesRoot.getModulePassword(SecurePreferencesRoot.java:259) at org.eclipse.equinox.internal.security.storage.SecurePreferencesRoot.getPassword(SecurePreferencesRoot.java:224) at org.eclipse.equinox.internal.security.storage.SecurePreferences.get(SecurePreferences.java:262) at org.eclipse.equinox.internal.security.storage.SecurePreferencesWrapper.get(SecurePreferencesWrapper.java:106) at org.eclipse.mylyn.tasks.core.TaskRepository.getAuthInfo(TaskRepository.java:402) - locked <0x692f62c8> (a java.lang.Object) at org.eclipse.mylyn.tasks.core.TaskRepository.getCredentials(TaskRepository.java:518) - locked <0x692cd268> (a org.eclipse.mylyn.tasks.core.TaskRepository) at org.eclipse.mylyn.tasks.core.TaskRepository.getUserName(TaskRepository.java:653) at org.eclipse.mylyn.tasks.core.TaskRepository.getUserName(TaskRepository.java:643) at org.eclipse.mylyn.internal.tasks.core.TaskRepositoryManager.isOwnedByUser(TaskRepositoryManager.java:423) at org.eclipse.mylyn.internal.tasks.core.TaskActivityManager.isCompletedToday(TaskActivityManager.java:616) at org.eclipse.mylyn.internal.tasks.ui.views.TaskListInterestFilter.shouldAlwaysShow(TaskListInterestFilter.java:98) at org.eclipse.mylyn.internal.tasks.ui.views.TaskListInterestFilter.select(TaskListInterestFilter.java:65) at org.eclipse.mylyn.internal.tasks.ui.views.TaskListContentProvider.filter(TaskListContentProvider.java:193) at org.eclipse.mylyn.internal.tasks.ui.views.TaskListContentProvider.selectContainer(TaskListContentProvider.java:126) at org.eclipse.mylyn.internal.tasks.ui.views.TaskListContentProvider.applyFilter(TaskListContentProvider.java:111) at org.eclipse.mylyn.internal.tasks.ui.views.TaskListContentProvider.getElements(TaskListContentProvider.java:55) at org.eclipse.jface.viewers.StructuredViewer.getRawChildren(StructuredViewer.java:989) at org.eclipse.jface.viewers.ColumnViewer.getRawChildren(ColumnViewer.java:703) at org.eclipse.jface.viewers.AbstractTreeViewer.getRawChildren(AbstractTreeViewer.java:1314) at org.eclipse.jface.viewers.TreeViewer.getRawChildren(TreeViewer.java:391) at org.eclipse.jface.viewers.StructuredViewer.getFilteredChildren(StructuredViewer.java:896) at org.eclipse.jface.viewers.AbstractTreeViewer.getSortedChildren(AbstractTreeViewer.java:601) at org.eclipse.jface.viewers.AbstractTreeViewer.updateChildren(AbstractTreeViewer.java:2563) at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefreshStruct(AbstractTreeViewer.java:1849) at org.eclipse.jface.viewers.TreeViewer.internalRefreshStruct(TreeViewer.java:717) at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefresh(AbstractTreeViewer.java:1824) at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefresh(AbstractTreeViewer.java:1781) at org.eclipse.jface.viewers.AbstractTreeViewer.internalRefresh(AbstractTreeViewer.java:1767) at org.eclipse.jface.viewers.StructuredViewer$7.run(StructuredViewer.java:1487) at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1422) at org.eclipse.jface.viewers.TreeViewer.preservingSelection(TreeViewer.java:403) at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1383) at org.eclipse.jface.viewers.StructuredViewer.refresh(StructuredViewer.java:1485) at org.eclipse.jface.viewers.ColumnViewer.refresh(ColumnViewer.java:537) at org.eclipse.ui.dialogs.FilteredTree$NotifyingTreeViewer.refresh(FilteredTree.java:1208) at org.eclipse.jface.viewers.StructuredViewer.refresh(StructuredViewer.java:1444) at org.eclipse.ui.dialogs.FilteredTree$NotifyingTreeViewer.refresh(FilteredTree.java:1198) at org.eclipse.jface.viewers.StructuredViewer.setSorter(StructuredViewer.java:1760) at org.eclipse.mylyn.internal.context.ui.actions.FocusTaskListAction$1.run(FocusTaskListAction.java:115) at org.eclipse.mylyn.internal.tasks.ui.util.TasksUiInternal.preservingSelection(TasksUiInternal.java:918) at org.eclipse.mylyn.internal.context.ui.actions.FocusTaskListAction.installInterestFilter(FocusTaskListAction.java:103) at org.eclipse.mylyn.context.ui.AbstractFocusViewAction.updateInterestFilter(AbstractFocusViewAction.java:418) at org.eclipse.mylyn.context.ui.AbstractFocusViewAction.valueChanged(AbstractFocusViewAction.java:302) at org.eclipse.mylyn.context.ui.AbstractFocusViewAction.update(AbstractFocusViewAction.java:275) at org.eclipse.mylyn.context.ui.AbstractFocusViewAction.update(AbstractFocusViewAction.java:267) at org.eclipse.mylyn.internal.context.ui.actions.FocusTaskListAction.init(FocusTaskListAction.java:61) at org.eclipse.ui.internal.ViewPluginAction.initDelegate(ViewPluginAction.java:59) at org.eclipse.ui.internal.PluginAction.createDelegate(PluginAction.java:125) at org.eclipse.ui.internal.PluginAction.selectionChanged(PluginAction.java:275) at org.eclipse.ui.internal.PartPluginAction.registerSelectionListener(PartPluginAction.java:40) at org.eclipse.ui.internal.ViewPluginAction.<init>(ViewPluginAction.java:38) at org.eclipse.ui.internal.ActionDescriptor.createAction(ActionDescriptor.java:259) at org.eclipse.ui.internal.ActionDescriptor.<init>(ActionDescriptor.java:176) at org.eclipse.ui.internal.ViewActionBuilder.createActionDescriptor(ViewActionBuilder.java:47) at org.eclipse.ui.internal.PluginActionBuilder.readElement(PluginActionBuilder.java:161) at org.eclipse.ui.internal.registry.RegistryReader.readElements(RegistryReader.java:144) at org.eclipse.ui.internal.registry.RegistryReader.readElementChildren(RegistryReader.java:133) at org.eclipse.ui.internal.PluginActionBuilder.readElement(PluginActionBuilder.java:144) at org.eclipse.ui.internal.registry.RegistryReader.readElements(RegistryReader.java:144) at org.eclipse.ui.internal.registry.RegistryReader.readExtension(RegistryReader.java:155) at org.eclipse.ui.internal.registry.RegistryReader.readRegistry(RegistryReader.java:176) at org.eclipse.ui.internal.PluginActionBuilder.readContributions(PluginActionBuilder.java:115) at org.eclipse.ui.internal.ViewActionBuilder.readActionExtensions(ViewActionBuilder.java:76) at org.eclipse.ui.internal.ViewReference.createPartHelper(ViewReference.java:401) at org.eclipse.ui.internal.ViewReference.createPart(ViewReference.java:229) at org.eclipse.ui.internal.WorkbenchPartReference.getPart(WorkbenchPartReference.java:595) at org.eclipse.ui.internal.PartPane.setVisible(PartPane.java:313) at org.eclipse.ui.internal.ViewPane.setVisible(ViewPane.java:529) at org.eclipse.ui.internal.presentations.PresentablePart.setVisible(PresentablePart.java:180) at org.eclipse.ui.internal.presentations.util.PresentablePartFolder.select(PresentablePartFolder.java:270) at org.eclipse.ui.internal.presentations.util.LeftToRightTabOrder.select(LeftToRightTabOrder.java:65) at org.eclipse.ui.internal.presentations.util.TabbedStackPresentation.selectPart(TabbedStackPresentation.java:473) at org.eclipse.ui.internal.PartStack.refreshPresentationSelection(PartStack.java:1254) at org.eclipse.ui.internal.PartStack.createControl(PartStack.java:666) at org.eclipse.ui.internal.PartStack.createControl(PartStack.java:574) at org.eclipse.ui.internal.PartSashContainer.createControl(PartSashContainer.java:568) at org.eclipse.ui.internal.PerspectiveHelper.activate(PerspectiveHelper.java:272) at org.eclipse.ui.internal.Perspective.onActivate(Perspective.java:981) at org.eclipse.ui.internal.WorkbenchPage.onActivate(WorkbenchPage.java:2632) at org.eclipse.ui.internal.WorkbenchWindow$27.run(WorkbenchWindow.java:2986) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ui.internal.WorkbenchWindow.setActivePage(WorkbenchWindow.java:2967) at org.eclipse.ui.internal.WorkbenchWindow$21.runWithException(WorkbenchWindow.java:2284) at org.eclipse.ui.internal.StartupThreading$StartupRunnable.run(StartupThreading.java:31) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:134) - locked <0x52e490d0> (a org.eclipse.swt.widgets.RunnableLock) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3515) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3162) at org.eclipse.ui.application.WorkbenchAdvisor.openWindows(WorkbenchAdvisor.java:803) at org.eclipse.ui.internal.Workbench$30.runWithException(Workbench.java:1553) at org.eclipse.ui.internal.StartupThreading$StartupRunnable.run(StartupThreading.java:31) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:134) - locked <0x6748b7d8> (a org.eclipse.swt.widgets.RunnableLock) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3515) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3162) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2509) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2399) at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:669) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:662) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:369) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:619) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:574) at org.eclipse.equinox.launcher.Main.run(Main.java:1407)
That's bad. Do you have any suggestion as what to do to fix it? I'm running a job in bundle's start. I could set some delay or switch to UIJob maybe?
I see that this is not a problem in the migration job but the side effect of the migration. So the task list synchronisation process should be somehow modified to accommodate for the changes.
Created attachment 168720 [details] Version 3 This one includes a fix for workbench hang during bootstraping. TaskRepository.getUsername doesn't have to access password so I rewrote it to not access full credentials.
That looks good to me. Can you include a test case for the migration?
Created attachment 168895 [details] Version 4 Added test. Code now migrates Proxy and HTTP credentials too.
Pawel, the migration had already been implemented as part of bug 237042 by Rob: https://bugs.eclipse.org/bugs/attachment.cgi?id=136619 . I'll put this on for 3.5 to revive those changes which we had to revert in the 3.2 stream to maintain compatibility with Eclipse 3.3.
Great. When will this be in HEAD?
I am targeting this for 3.5.
We'll need to take this on for 3.7 since e4 does no longer ship the old compatibility support for storing credentials (bug 336129).
Due to a lack of time I'm removing this from the milestone. The work-around that was committed as part of bug 357205 seems to work reasonably well.
9668: 278474: store TaskRepository credentials in SecureCredentialsStore [I93daacac] https://git.eclipse.org/r/#/c/9668/ Since this uses a different node in the secure store, we should probably migrate the credentials of repositories that had set PROPERTY_USE_SECURE_STORAGE to true.
The review looks good. What I'm missing though is code that migrates existing credentials. At best this should be transparent for users, i.e. if credentials are accessed for the first time they should be migrated: * from the legacy keyring * from the existing secure store node to the org.eclipse.mylyn.commmons.repostiory node
I pushed a smaller fix for Bugzilla to avoid opening the keyring when the editor is opened: https://git.eclipse.org/r/#/c/12584/.
Sam, there a bunch of test failures that need to be addressed as well: https://hudson.eclipse.org/sandbox/job/mylyn-tasks-gerrit/534/.
I had one more thought on this: I like how you used the ICredentialsStore service. Taking that one step further, do you think it would be feasible to expose a TaskRepository as a RepositoryLocation? This would be useful when migrating to new transport layers and such. I'm thinking that we TaskRepository should possibly use CredentialsFactory so username/password pairs end up being stored in the same location that RepositoryLocation would use. What do you think?
Moving to 3.10 since it's a major change and we are nearing the end of the release cycle already.
(In reply to comment #19) > I had one more thought on this: I like how you used the ICredentialsStore > service. Taking that one step further, do you think it would be feasible to > expose a TaskRepository as a RepositoryLocation? This would be useful when > migrating to new transport layers and such. > > I'm thinking that we TaskRepository should possibly use CredentialsFactory so > username/password pairs end up being stored in the same location that > RepositoryLocation would use. What do you think? I think we might be able to make that work, but we'd need to change the key to include both the URL and username in order to fix bug 414907, so it wouldn't be exactly the same location as RepositoryLocation (I don't want to tackle fixing RepositoryLocation as part of this).
We now have the following reviews. Leo, Steffen, it would be great if you could take a look. 9668: 278474: store TaskRepository credentials in SecureCredentialsStore [I93daacac] https://git.eclipse.org/r/#/c/9668/ 21364: create class to represent headless credentials [I7f43f529] https://git.eclipse.org/r/#/c/21364/ 21365: cleanup boolean properties in TaskRepository [Icf58826e] https://git.eclipse.org/r/#/c/21365/ 21366: 278474: perform migration of credentials to secure storage [I590cabb6] https://git.eclipse.org/r/#/c/21366/
Thanks Steffen. I've updated the reviews. The builds will all fail due to 427942: builds failing due to "Could not create temp dir" error https://bugs.eclipse.org/bugs/show_bug.cgi?id=427942
I have retriggered all builds.
The changes have been merged.
Frank, unfortunately the tests in org.eclipse.mylyn.bugzilla.rest.tests.tck.ValidationTest are failing because of this change. Sorry I didn't see that before this was merged. It looks like it's because the repository passed to validateRepository is not the same one that had the credentials set on it, so it doesn't have the enabled flag set. I have seen so many test failures due to using different repository instances in different parts of the test and they are often very hard to debug. I think tests should normally use the same TaskRepository object throughout the test. This will make the tests more stable and also more realistic.
(In reply to comment #26) > Frank, unfortunately the tests in > org.eclipse.mylyn.bugzilla.rest.tests.tck.ValidationTest are failing because of > this change. Sorry I didn't see that before this was merged. It looks like it's > because the repository passed to validateRepository is not the same one that had > the credentials set on it, so it doesn't have the enabled flag set. > > I have seen so many test failures due to using different repository instances in > different parts of the test and they are often very hard to debug. I think tests > should normally use the same TaskRepository object throughout the test. This > will make the tests more stable and also more realistic. Create review https://git.eclipse.org/r/22254 for a fix.
Created attachment 240117 [details] mylyn/context/zip
I restarted Eclipse with an open Bugzilla editor (and activate task) after upgrading to the latest weekly and all my passwords (Bugzilla, Gerrit) were lost in that workspace. I did see the migration job running in the progress view after the restart.
Was there anything in your error log? Can you try restarting again?
No, there wasn't any error in the log. When I set the password manually though I noticed this: org.eclipse.equinox.security.storage.StorageException: No password provided. at org.eclipse.equinox.internal.security.storage.SecurePreferencesRoot.getModulePassword(SecurePreferencesRoot.java:304) at org.eclipse.equinox.internal.security.storage.SecurePreferencesRoot.getPassword(SecurePreferencesRoot.java:224) at org.eclipse.equinox.internal.security.storage.SecurePreferences.put(SecurePreferences.java:224) at org.eclipse.equinox.internal.security.storage.SecurePreferencesWrapper.put(SecurePreferencesWrapper.java:110) at org.eclipse.mylyn.internal.commons.repositories.core.SecureCredentialsStore.put(SecureCredentialsStore.java:135) at org.eclipse.mylyn.internal.commons.repositories.core.SecureCredentialsStore.put(SecureCredentialsStore.java:129) at org.eclipse.mylyn.tasks.core.TaskRepository.addAuthInfo(TaskRepository.java:270) at org.eclipse.mylyn.tasks.core.TaskRepository.setCredentials(TaskRepository.java:644) at org.eclipse.mylyn.tasks.ui.wizards.AbstractRepositorySettingsPage.applyTo(AbstractRepositorySettingsPage.java:1734) at org.eclipse.mylyn.internal.bugzilla.ui.tasklist.BugzillaRepositorySettingsPage.applyToInternal(BugzillaRepositorySettingsPage.java:665) at org.eclipse.mylyn.internal.bugzilla.ui.tasklist.BugzillaRepositorySettingsPage.applyTo(BugzillaRepositorySettingsPage.java:556) at org.eclipse.mylyn.tasks.ui.wizards.AbstractTaskRepositoryPage.performFinish(AbstractTaskRepositoryPage.java:283) I have no idea what caused the secure store to get corrupted. I was able to recover by clearing the secure store and setting passwords again. I was a bit surprised that the credentials migration job ran again when I restarted Eclipse after clearing the secure store. I think part of the problem is that we don't show anything in the UI when persisting of credentials fails. It's difficult for users to understand what's going on in that case. This isn't a new problem but the secure store seems more likely to fail than the old keyring so this we'll likely see these failures more often now.
Hmm, it seems the migratoin job runs on every startup (it only migrates the credentials the first time it runs). I'll fix it so the job only runs if it actually has something to do. Thanks for pointing it out. It sounds like it's not clear that the secure store corruption is related to the migration.
(In reply to comment #32) > It sounds like it's not clear that the secure store corruption is related to the > migration. No, I don't think it is. I have seen this happen fairly frequently on my Mac. It could be related to the native integration or the keystore getting shared between different Eclipse intances/versions (3.8 and 4.4).
