Add possibility to remember password for shared databases. #1846

obraliar · 2016-08-25T01:24:52Z

Issue: #1703. (completition)

Store last used connection in the "open remote database dialog"

Screenshot:

Screenshots added (for bigger UI changes)
Manually tested changed features in running JabRef

koppor · 2016-08-25T11:23:07Z

src/main/java/net/sf/jabref/shared/prefs/SharedDatabasePreferences.java

+    private static final String SHARED_DATABASE_REMEMBER_PASSWORD = "sharedDatabaseRememberPassword";
+
+    // This {@link Preferences} is used only for things which should not appear in real JabRefPreferences due to security reasons.
+    private final Preferences internalPrefs = Preferences.userNodeForPackage(OpenSharedDatabaseDialog.class);


Is it possible to use another node for storing? The whole preference tree gets exported. Therefore, this should be located somewhere else @matthiasgeiger

koppor · 2016-08-25T11:23:44Z

Could you use some unique system properties such as the login name, serial number of Windows or something as additional encryption key? - If you use that too, maybe the preferences can be exported completely.

As alternative, on Windows, maybe the Windows Credential Store can be used? On Linux, the key store. On Mac?

matthiasgeiger · 2016-08-25T11:31:41Z

Maybe this Preferences userPrefs = Preferences.userRoot().node( "/com/tutego/insel" ); as used here http://openbook.rheinwerk-verlag.de/javainsel/javainsel_11_009.html could be used to create another registry node just for the password which is then not exported by JabRef.

// Edit: Of course it must be outside "/net/sf/jabref" ... ;-)

boceckts · 2016-08-25T11:39:29Z

src/main/java/net/sf/jabref/gui/shared/OpenSharedDatabaseDialog.java

@@ -194,6 +197,12 @@ private void applyGlobalPrefs() {
        if (sharedDatabaseUser.isPresent()) {
            userField.setText(sharedDatabaseUser.get());
        }
+
+        if (sharedDatabasePassword.isPresent()) {


The password should only be set when there is a password available and the checkbox to remember the password is checked so please add && sharedDatabaseRememberPassword.

See: https://github.com/JabRef/jabref/pull/1846/files#diff-bf287bbdc594ec6ec66a15154e141110R316. If the checkbox is not select the pref is going to be cleared.

boceckts · 2016-08-25T12:08:45Z

Only minor coments from me. Once they are done and the conflicts are reseolved, it looks good to me 👍

Siedlerchr · 2016-08-25T12:41:10Z

Password Hashing is the solution! (It should have come earlier to my mind)
https://paragonie.com/blog/2016/02/how-safely-store-password-in-2016#java

obraliar · 2016-08-25T13:59:26Z

@matthiasgeiger Unfortunately it does not work. I used for testing private final Preferences internalPrefs = Preferences.userRoot().node("/bla/bla1/security"); If I export the preferences, the following nodes appear again:

  1 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
  2 <!DOCTYPE preferences SYSTEM "http://java.sun.com/dtd/preferences.dtd">
  3 <preferences EXTERNAL_XML_VERSION="1.0">
  4   <root type="user">
  5     <map/>
  6     <node name="net">
  7       <map/>
  8       <node name="sf">
  9         <map/>
 10         <node name="jabref">
 11           <map>
 12           <!-- ... -->
 13           </map>
 14           <node name="bibtexKeyPattern">
 15             <map/>
 16           </node>
 17           <node name="bibtexkeypatterns">
 18             <map/>
 19           </node>
 20           <node name="gui">
 21             <map/>
 22             <node name="shared">
 23               <map>
 24                 <entry key="sharedDatabasePassword" value="kJicIWFbSjTjw5QYiENFLA=="/>
 25               </map>
 26             </node>
 27           </node>
 28           <node name="labelPattern">
 29             <map/>
 30           </node>
 31           <node name="logic">
 32             <map/>
 33             <node name="labelpattern">
 34               <map/>
 35             </node>
 36           </node>
 37         </node>
 38       </node>
 39     </node>
 40   </root>
 41 </preferences>

As another solution we could add new SharedDatabasePreferences().clear(); into JabRefPreferences.exportPreferences(String filename).

matthiasgeiger · 2016-08-25T14:05:09Z

@obraliar I think that the exported key is a remnant of previously saved settings.

If you are using windows use regedit to delete the keys in the node HKEY_CURRENT_USER\SOFTWARE\JavaSoft\Prefs\net\sf\jabref\gui\shared

Your addition should be placed in HKEY_CURRENT_USER\SOFTWARE\JavaSoft\Prefs\bla\bla1\security

obraliar · 2016-08-25T14:06:24Z

@Siedlerchr There should be a possibility to decrypt the password again as this is going to be sent to DBMS. Could you explain what to do with hashing?

obraliar · 2016-08-25T14:14:30Z

@matthiasgeiger 👍 I'm using Linux (Debian) and there you have to remove ~/.java/.userPrefs/net/sf from file system. Now it works very well!
Now using: private final Preferences internalPrefs = Preferences.userRoot().node("/net/sf/security/jabref");

obraliar · 2016-08-25T14:19:57Z

@koppor What about using the MAC address?

matthiasgeiger · 2016-08-25T14:27:23Z

I think the suggestion is now obsolote as exporting the prefs without the password is now possible. Maybe we should add a comment in the help file that the password is saved on the local hard disk - then a user can decide whether he wants to save it or not...

Regarding the prefs node: As we want to move the package from "net.sf" to "org" eventually in the future it would be perhaps better to use Preferences.getUserNodeForPackage(JabRefMain.class).parent().node("jabref-pwdstorage") to use a relative approach.

obraliar · 2016-08-25T14:39:26Z

@matthiasgeiger I agree cause the safest way is not to use the function "remember password" as this is generally a security gap due to symetric encryption.

Siedlerchr · 2016-08-25T16:45:57Z

Forget about the Hashing. I didn't think about that you need it in plaintex to send to the db..

koppor · 2016-08-26T00:00:22Z

@matthiasgeiger I would nevertheless keep a simple encryption to make it a bit harder to access the password.

obraliar · 2016-08-29T13:49:10Z

@JabRef/developers Could someone look at this PR and possibly merge it in, please?
Reason: @koppor is offline for a week.

stefan-kolb · 2016-08-29T14:00:16Z

LGTM 👍

Siedlerchr · 2016-08-29T15:13:56Z

src/main/java/net/sf/jabref/gui/shared/OpenSharedDatabaseDialog.java


 public class OpenSharedDatabaseDialog extends JDialog {

+    private static final Log LOGGER = LogFactory.getLog(Password.class);


Change the class in getLog to the actual class name (OpenSharedDatabaseDialog)
Otherwise logs shows wrong class.

Siedlerchr · 2016-08-29T15:45:55Z

Generally good work, if you address the comments, then we can merge it in 👍

stefan-kolb · 2016-09-02T08:54:26Z

src/main/java/net/sf/jabref/gui/shared/OpenSharedDatabaseDialog.java


 public class OpenSharedDatabaseDialog extends JDialog {

+    protected static final Log LOGGER = LogFactory.getLog(OpenSharedDatabaseDialog.class);


Why is the logger protected here?

Done. Copy & paste from DBMSProcessor 🙈.

- Add checkbox to the open shared database dialog - Add SharedDatabasePreferences - Add password encrypting and decrypting methods - Update localization entries - Reorganize clearing methods for Preferences

- Use username as one operand - Add new parameter to the constructor

- Use CBC and padding - Hash the key before using - Simplify conversion

obraliar · 2016-09-04T21:08:38Z

All issues have been fixed.

* Check integrity edition check * Conflict in Jabref_fr * Changed order Biblatex/Bibtex condition * Create own object * Rename variable * Extract checker to own file * Improved comment * French localization: Jabref_fr: empty strings translated + removal of unused header (#1911) * Remove teamscale findings in the database package (#1577) * Check integrity edition check * Changed order Biblatex/Bibtex condition * Create own object * Rename variable * Mark some methods as deprecated in BibEntry and BibDatabase (#1913) * Mark some methods as deprecated in BibEntry and BibDatabase * Rename getResolvedFieldOrAlias * Use flatmap * Fix location field not exported correctly to office 2007 xml (#1909) * Fix location field not exported to office 2007 xml * Add some test for exporting location and address field Address in xml field is now imported as location add some javadoc * Add possibility to remember password for shared databases. (#1846) * Add possibility to remember password. - Add checkbox to the open shared database dialog - Add SharedDatabasePreferences - Add password encrypting and decrypting methods - Update localization entries - Reorganize clearing methods for Preferences * Change prefs node and add class comment. * Relativate node path for password storage. * Fix LOGGER factory. * Improve password encryption using XOR. - Use username as one operand - Add new parameter to the constructor * Extract method. * Improve exception handling. * Improve password encryption and decryption. - Use CBC and padding - Hash the key before using - Simplify conversion * Fix modifier. Fix conflicts. * Extract checker to own file * Improved comment * Translation of shared (#1919) * Add missing entries in german localization * Resolved conflicts * Some more conflicts de sv * Expressions changed

obraliar force-pushed the PasswordManagementForSharedDatabase branch from 5079221 to b38f020 Compare August 25, 2016 01:55

koppor added stupro-ready-for-internal-review labels Aug 25, 2016

koppor added this to the v3.6 milestone Aug 25, 2016

koppor reviewed Aug 25, 2016
View reviewed changes

koppor modified the milestones: v3.7, v3.6 Aug 25, 2016

boceckts reviewed Aug 25, 2016
View reviewed changes

boceckts removed the stupro-ready-for-internal-review label Aug 25, 2016

obraliar force-pushed the PasswordManagementForSharedDatabase branch from b38f020 to 55de86b Compare August 25, 2016 14:20

obraliar force-pushed the PasswordManagementForSharedDatabase branch from 55de86b to 2b94985 Compare August 25, 2016 14:30

obraliar force-pushed the PasswordManagementForSharedDatabase branch 3 times, most recently from 316e94f to 2e700f6 Compare August 25, 2016 16:28

obraliar changed the title ~~[WIP] Add possibility to remember password for shared databases.~~ Add possibility to remember password for shared databases. Aug 25, 2016

Braunch changed the title ~~Add possibility to remember password for shared databases.~~ [WIP]Add possibility to remember password for shared databases. Aug 29, 2016

Braunch changed the title ~~[WIP]Add possibility to remember password for shared databases.~~ Add possibility to remember password for shared databases. Aug 29, 2016

obraliar force-pushed the PasswordManagementForSharedDatabase branch from f459b5c to d5fec26 Compare August 29, 2016 13:53

Siedlerchr reviewed Aug 29, 2016
View reviewed changes

obraliar force-pushed the PasswordManagementForSharedDatabase branch from d5fec26 to 5ae1bfa Compare September 2, 2016 00:23

stefan-kolb reviewed Sep 2, 2016
View reviewed changes

obraliar force-pushed the PasswordManagementForSharedDatabase branch from 5ae1bfa to 2ec2e0d Compare September 2, 2016 13:37

obraliar added 8 commits September 4, 2016 18:27

Add possibility to remember password.

1c99a8f

- Add checkbox to the open shared database dialog - Add SharedDatabasePreferences - Add password encrypting and decrypting methods - Update localization entries - Reorganize clearing methods for Preferences

Change prefs node and add class comment.

6390dab

Relativate node path for password storage.

bf6d431

Fix LOGGER factory.

879fb22

Improve password encryption using XOR.

09a41c2

- Use username as one operand - Add new parameter to the constructor

Extract method.

aa719b8

Improve exception handling.

a38ff25

Improve password encryption and decryption.

f9edc2e

- Use CBC and padding - Hash the key before using - Simplify conversion

obraliar force-pushed the PasswordManagementForSharedDatabase branch from 2ec2e0d to 968e8b1 Compare September 4, 2016 16:35

Fix modifier. Fix conflicts.

0a8fc3a

obraliar force-pushed the PasswordManagementForSharedDatabase branch from 968e8b1 to 0a8fc3a Compare September 4, 2016 20:39

stefan-kolb merged commit 3f64b08 into JabRef:master Sep 4, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add possibility to remember password for shared databases. #1846

Add possibility to remember password for shared databases. #1846

obraliar commented Aug 25, 2016 •

edited

Loading

koppor Aug 25, 2016

obraliar Aug 25, 2016

koppor commented Aug 25, 2016 •

edited

Loading

matthiasgeiger commented Aug 25, 2016 •

edited

Loading

boceckts Aug 25, 2016

obraliar Aug 25, 2016

boceckts commented Aug 25, 2016

Siedlerchr commented Aug 25, 2016

obraliar commented Aug 25, 2016 •

edited by koppor

Loading

matthiasgeiger commented Aug 25, 2016 •

edited

Loading

obraliar commented Aug 25, 2016

obraliar commented Aug 25, 2016

obraliar commented Aug 25, 2016

matthiasgeiger commented Aug 25, 2016 •

edited

Loading

obraliar commented Aug 25, 2016 •

edited

Loading

Siedlerchr commented Aug 25, 2016

koppor commented Aug 26, 2016

obraliar commented Aug 29, 2016

stefan-kolb commented Aug 29, 2016

Siedlerchr Aug 29, 2016

obraliar Sep 2, 2016

Siedlerchr commented Aug 29, 2016

stefan-kolb Sep 2, 2016

obraliar Sep 2, 2016

obraliar commented Sep 4, 2016


		public class OpenSharedDatabaseDialog extends JDialog {

		private static final Log LOGGER = LogFactory.getLog(Password.class);


		public class OpenSharedDatabaseDialog extends JDialog {

		protected static final Log LOGGER = LogFactory.getLog(OpenSharedDatabaseDialog.class);

Add possibility to remember password for shared databases. #1846

Add possibility to remember password for shared databases. #1846

Conversation

obraliar commented Aug 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Aug 25, 2016 • edited Loading

matthiasgeiger commented Aug 25, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boceckts commented Aug 25, 2016

Siedlerchr commented Aug 25, 2016

obraliar commented Aug 25, 2016 • edited by koppor Loading

matthiasgeiger commented Aug 25, 2016 • edited Loading

obraliar commented Aug 25, 2016

obraliar commented Aug 25, 2016

obraliar commented Aug 25, 2016

matthiasgeiger commented Aug 25, 2016 • edited Loading

obraliar commented Aug 25, 2016 • edited Loading

Siedlerchr commented Aug 25, 2016

koppor commented Aug 26, 2016

obraliar commented Aug 29, 2016

stefan-kolb commented Aug 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Siedlerchr commented Aug 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obraliar commented Sep 4, 2016

obraliar commented Aug 25, 2016 •

edited

Loading

koppor commented Aug 25, 2016 •

edited

Loading

matthiasgeiger commented Aug 25, 2016 •

edited

Loading

obraliar commented Aug 25, 2016 •

edited by koppor

Loading

matthiasgeiger commented Aug 25, 2016 •

edited

Loading

matthiasgeiger commented Aug 25, 2016 •

edited

Loading

obraliar commented Aug 25, 2016 •

edited

Loading