From ff17a1d6373a20d48605cb3afa32566144044759 Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Sat, 13 Oct 2018 16:44:37 -0400 Subject: [PATCH] Fix file import & sane format migration. --- platform-darwin/External/Pearl | 2 +- .../gui/view/UserContentPanel.java | 5 +- .../masterpassword/model/impl/MPFileUser.java | 93 +++++++++++++------ .../model/impl/MPFileUserManager.java | 36 ++++--- .../model/impl/MPFlatUnmarshaller.java | 52 ++++++----- .../masterpassword/model/impl/MPJSONFile.java | 2 +- .../model/impl/MPMarshalFormat.java | 7 ++ public/site | 2 +- 8 files changed, 131 insertions(+), 68 deletions(-) diff --git a/platform-darwin/External/Pearl b/platform-darwin/External/Pearl index bc737d41..3d04d775 160000 --- a/platform-darwin/External/Pearl +++ b/platform-darwin/External/Pearl @@ -1 +1 @@ -Subproject commit bc737d41fa15c9e4d05e7f73c25995da67611e52 +Subproject commit 3d04d775e01ab1a437bb42166e92662ffffeedd4 diff --git a/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java b/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java index 3e656e7c..222c4967 100644 --- a/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java +++ b/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java @@ -3,7 +3,7 @@ package com.lyndir.masterpassword.gui.view; import static com.lyndir.lhunath.opal.system.util.StringUtils.*; import com.google.common.base.*; -import com.google.common.collect.*; +import com.google.common.collect.ImmutableList; import com.google.common.primitives.UnsignedInteger; import com.google.common.util.concurrent.ListenableFuture; import com.lyndir.lhunath.opal.system.logging.Logger; @@ -407,6 +407,9 @@ public class UserContentPanel extends JPanel implements MasterPassword.Listener, Res.job( () -> { try { user.authenticate( masterPassword ); + + if (user instanceof MPFileUser) + ((MPFileUser) user).migrateTo( MPMarshalFormat.DEFAULT ); } catch (final MPIncorrectMasterPasswordException e) { logger.wrn( e, "During user authentication for: %s", user ); diff --git a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java index 8434a575..c7d8a990 100755 --- a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java +++ b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java @@ -24,7 +24,6 @@ import com.lyndir.masterpassword.model.MPIncorrectMasterPasswordException; import com.lyndir.masterpassword.model.MPUser; import java.io.File; import java.io.IOException; -import java.util.Objects; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.joda.time.Instant; @@ -41,7 +40,7 @@ public class MPFileUser extends MPBasicUser { @Nullable private byte[] keyID; - private File path; + private File file; private MPMarshalFormat format; private MPMarshaller.ContentMode contentMode; @@ -54,33 +53,37 @@ public class MPFileUser extends MPBasicUser { public static MPFileUser load(final File file) throws IOException, MPMarshalException { for (final MPMarshalFormat format : MPMarshalFormat.values()) - if (file.getName().endsWith( format.fileSuffix() )) + if (format.matches(file)) return format.unmarshaller().readUser( file ); return null; } - public MPFileUser(final String fullName, final File path) { - this( fullName, null, MPAlgorithm.Version.CURRENT.getAlgorithm(), path ); + public MPFileUser(final String fullName, final File location) { + this( fullName, null, MPAlgorithm.Version.CURRENT.getAlgorithm(), location ); } - public MPFileUser(final String fullName, @Nullable final byte[] keyID, final MPAlgorithm algorithm, final File path) { + public MPFileUser(final String fullName, @Nullable final byte[] keyID, final MPAlgorithm algorithm, final File location) { this( fullName, keyID, algorithm, 0, null, new Instant(), false, - MPMarshaller.ContentMode.PROTECTED, MPMarshalFormat.DEFAULT, path ); + MPMarshaller.ContentMode.PROTECTED, MPMarshalFormat.DEFAULT, location ); } public MPFileUser(final String fullName, @Nullable final byte[] keyID, final MPAlgorithm algorithm, final int avatar, @Nullable final MPResultType defaultType, final ReadableInstant lastUsed, final boolean hidePasswords, - final MPMarshaller.ContentMode contentMode, final MPMarshalFormat format, final File path) { + final MPMarshaller.ContentMode contentMode, final MPMarshalFormat format, final File location) { super( avatar, fullName, algorithm ); this.keyID = (keyID != null)? keyID.clone(): null; this.defaultType = (defaultType != null)? defaultType: algorithm.mpw_default_result_type(); this.lastUsed = lastUsed; this.hidePasswords = hidePasswords; - this.path = path; this.format = format; this.contentMode = contentMode; + + if (location.isDirectory()) + this.file = new File( location, getFullName() + getFormat().fileSuffix() ); + else + this.file = location; } @Nullable @@ -89,10 +92,6 @@ public class MPFileUser extends MPBasicUser { return (keyID == null)? null: keyID.clone(); } - public void setPath(final File path) { - this.path = path; - } - @Override public void setAlgorithm(final MPAlgorithm algorithm) { if (!algorithm.equals( getAlgorithm() ) && (keyID != null)) { @@ -117,20 +116,12 @@ public class MPFileUser extends MPBasicUser { return format; } - public void setFormat(final MPMarshalFormat format) { - if (Objects.equals( this.format, format )) - return; - - this.format = format; - setChanged(); - } - public MPMarshaller.ContentMode getContentMode() { return contentMode; } public void setContentMode(final MPMarshaller.ContentMode contentMode) { - if (Objects.equals( this.contentMode, contentMode )) + if (this.contentMode == contentMode) return; this.contentMode = contentMode; @@ -143,7 +134,7 @@ public class MPFileUser extends MPBasicUser { } public void setDefaultType(final MPResultType defaultType) { - if (Objects.equals( this.defaultType, defaultType )) + if (this.defaultType == defaultType) return; this.defaultType = defaultType; @@ -180,7 +171,47 @@ public class MPFileUser extends MPBasicUser { } public File getFile() { - return new File( path, getFullName() + getFormat().fileSuffix() ); + return file; + } + + public void migrateTo(final MPMarshalFormat format) { + if (this.format == format) + return; + + migrateTo( file.getParentFile(), format ); + } + + public void migrateTo(final File path) { + migrateTo( path, format ); + } + + /** + * Move the file for this user to the given path using a standard user-derived filename (ie. {@code [full name].[format suffix]}) + * + * The user's old file is either moved to the new or deleted. If the user's file was already at the destination, it doesn't change. + * If a file already exists at the destination, it is overwritten. + */ + public void migrateTo(final File path, final MPMarshalFormat newFormat) { + MPMarshalFormat oldFormat = format; + File oldFile = file, newFile = new File( path, getFullName() + newFormat.fileSuffix() ); + + // If the format hasn't changed, migrate by moving the file: the contents doesn't need to change. + if ((oldFormat == newFormat) && !oldFile.equals( newFile ) && oldFile.exists()) + if (!oldFile.renameTo( newFile )) + logger.wrn( "Couldn't move %s to %s for migration.", oldFile, newFile ); + + this.format = newFormat; + this.file = newFile; + + // If the format has changed, save the new format into the new file and delete the old file. Revert if the user cannot be saved. + if ((oldFormat != newFormat) && !oldFile.equals( newFile )) + if (save()) { + if (oldFile.exists() && !oldFile.delete()) + logger.wrn( "Couldn't delete %s after migration.", oldFile ); + } else { + this.format = oldFormat; + this.file = oldFile; + } } @Override @@ -201,10 +232,16 @@ public class MPFileUser extends MPBasicUser { } } - public void save() { + /** + * @return {@code false} if the user is not fully loaded (complete), authenticated, or an issue prevented the marshalling. + */ + public boolean save() { + if (!isComplete()) + return false; + try { - if (isComplete()) - getFormat().marshaller().marshall( this ); + getFormat().marshaller().marshall( this ); + return true; } catch (final MPKeyUnavailableException e) { logger.wrn( e, "Cannot write out changes for unauthenticated user: %s.", this ); @@ -212,6 +249,8 @@ public class MPFileUser extends MPBasicUser { catch (final IOException | MPMarshalException | MPAlgorithmException e) { logger.err( e, "Unable to write out changes for user: %s", this ); } + + return false; } @Override diff --git a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUserManager.java b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUserManager.java index edb58c41..d7b9dda6 100644 --- a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUserManager.java +++ b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUserManager.java @@ -78,18 +78,17 @@ public class MPFileUserManager { return; } - for (final File file : pathFiles) - try { - MPFileUser user = MPFileUser.load( file ); - if (user != null) { - MPFileUser previousUser = userByName.put( user.getFullName(), user ); - if ((previousUser != null) && (previousUser.getFormat().ordinal() > user.getFormat().ordinal())) - userByName.put( previousUser.getFullName(), previousUser ); - } - } - catch (final IOException | MPMarshalException e) { - logger.err( e, "Couldn't read user from: %s", file ); - } + for (final MPMarshalFormat format : MPMarshalFormat.values()) + for (final File file : pathFiles) + if (format.matches( file )) + try { + MPFileUser user = MPFileUser.load( file ); + if (user != null) + add( user ); + } + catch (final IOException | MPMarshalException e) { + logger.err( e, "Couldn't read user from: %s", file ); + } fireUpdated(); } @@ -99,12 +98,19 @@ public class MPFileUserManager { } public MPFileUser add(final MPFileUser user) { - user.setPath( getPath() ); - user.save(); + // We migrate in two steps to allow the first to complete even if the user is not in the right state to complete the latter. + user.migrateTo( getPath() ); + user.migrateTo( MPMarshalFormat.DEFAULT ); MPFileUser oldUser = userByName.put( user.getFullName(), user ); - if (oldUser != null) + if (oldUser != null) { oldUser.invalidate(); + + // Delete old user, it is replaced by the new one. + if (!oldUser.getFile().equals( user.getFile() ) && oldUser.getFile().exists()) + if (!oldUser.getFile().delete()) + logger.err( "Couldn't delete file: %s, after replacing with: %s", oldUser.getFile(), user.getFile() ); + } fireUpdated(); return user; diff --git a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFlatUnmarshaller.java b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFlatUnmarshaller.java index ba6ae53f..9d6e290f 100644 --- a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFlatUnmarshaller.java +++ b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFlatUnmarshaller.java @@ -25,8 +25,8 @@ import com.lyndir.lhunath.opal.system.CodeUtils; import com.lyndir.lhunath.opal.system.logging.Logger; import com.lyndir.lhunath.opal.system.util.ConversionUtils; import com.lyndir.masterpassword.*; -import com.lyndir.masterpassword.model.MPModelConstants; import com.lyndir.masterpassword.model.MPIncorrectMasterPasswordException; +import com.lyndir.masterpassword.model.MPModelConstants; import java.io.*; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -56,6 +56,7 @@ public class MPFlatUnmarshaller implements MPUnmarshaller { int mpVersion = 0, avatar = 0; boolean clearContent = false, headerStarted = false; MPResultType defaultType = null; + Instant date = null; //noinspection HardcodedLineSeparator for (final String line : CharStreams.readLines( reader )) @@ -66,10 +67,11 @@ public class MPFlatUnmarshaller implements MPUnmarshaller { headerStarted = true; else if ((fullName != null) && (keyID != null)) // Ends the header. - return new MPFileUser( fullName, keyID, MPAlgorithm.Version.fromInt( mpVersion ).getAlgorithm(), - avatar, defaultType, new Instant( 0 ), false, - clearContent? MPMarshaller.ContentMode.VISIBLE: MPMarshaller.ContentMode.PROTECTED, - MPMarshalFormat.Flat, file.getParentFile() ); + return new MPFileUser( + fullName, keyID, MPAlgorithm.Version.fromInt( mpVersion ).getAlgorithm(), avatar, defaultType, + date, false, clearContent? MPMarshaller.ContentMode.VISIBLE: MPMarshaller.ContentMode.PROTECTED, + MPMarshalFormat.Flat, file + ); } // Comment. @@ -91,6 +93,8 @@ public class MPFlatUnmarshaller implements MPUnmarshaller { clearContent = "visible".equalsIgnoreCase( value ); else if ("Default Type".equalsIgnoreCase( name )) defaultType = MPResultType.forType( ConversionUtils.toIntegerNN( value ) ); + else if ("Date".equalsIgnoreCase( name )) + date = MPModelConstants.dateTimeFormatter.parseDateTime( value ).toInstant(); } } } @@ -150,28 +154,32 @@ public class MPFlatUnmarshaller implements MPUnmarshaller { MPFileSite site; switch (importFormat) { case 0: - site = new MPFileSite( user, // - siteMatcher.group( 5 ), MPAlgorithm.Version.fromInt( ConversionUtils.toIntegerNN( - colon.matcher( siteMatcher.group( 4 ) ).replaceAll( "" ) ) ).getAlgorithm(), - user.getAlgorithm().mpw_default_counter(), - MPResultType.forType( ConversionUtils.toIntegerNN( siteMatcher.group( 3 ) ) ), - clearContent? null: siteMatcher.group( 6 ), - null, null, null, ConversionUtils.toIntegerNN( siteMatcher.group( 2 ) ), - MPModelConstants.dateTimeFormatter.parseDateTime( siteMatcher.group( 1 ) ).toInstant() ); + site = new MPFileSite( + user, siteMatcher.group( 5 ), + MPAlgorithm.Version.fromInt( ConversionUtils.toIntegerNN( + colon.matcher( siteMatcher.group( 4 ) ).replaceAll( "" ) ) ).getAlgorithm(), + user.getAlgorithm().mpw_default_counter(), + MPResultType.forType( ConversionUtils.toIntegerNN( siteMatcher.group( 3 ) ) ), + clearContent? null: siteMatcher.group( 6 ), + null, null, null, ConversionUtils.toIntegerNN( siteMatcher.group( 2 ) ), + MPModelConstants.dateTimeFormatter.parseDateTime( siteMatcher.group( 1 ) ).toInstant() ); if (clearContent) site.setSitePassword( site.getResultType(), siteMatcher.group( 6 ) ); break; case 1: - site = new MPFileSite( user, // - siteMatcher.group( 7 ), MPAlgorithm.Version.fromInt( ConversionUtils.toIntegerNN( - colon.matcher( siteMatcher.group( 4 ) ).replaceAll( "" ) ) ).getAlgorithm(), - UnsignedInteger.valueOf( colon.matcher( siteMatcher.group( 5 ) ).replaceAll( "" ) ), - MPResultType.forType( ConversionUtils.toIntegerNN( siteMatcher.group( 3 ) ) ), - clearContent? null: siteMatcher.group( 8 ), - MPResultType.GeneratedName, clearContent? null: siteMatcher.group( 6 ), null, - ConversionUtils.toIntegerNN( siteMatcher.group( 2 ) ), - MPModelConstants.dateTimeFormatter.parseDateTime( siteMatcher.group( 1 ) ).toInstant() ); + site = new MPFileSite( + user, siteMatcher.group( 7 ), + MPAlgorithm.Version.fromInt( ConversionUtils.toIntegerNN( + colon.matcher( siteMatcher.group( 4 ) ).replaceAll( "" ) ) ).getAlgorithm(), + UnsignedInteger.valueOf( + colon.matcher( siteMatcher.group( 5 ) ).replaceAll( "" ) ), + MPResultType.forType( ConversionUtils.toIntegerNN( siteMatcher.group( 3 ) ) ), + clearContent? null: siteMatcher.group( 8 ), + MPResultType.GeneratedName, + clearContent? null: siteMatcher.group( 6 ), + null, ConversionUtils.toIntegerNN( siteMatcher.group( 2 ) ), + MPModelConstants.dateTimeFormatter.parseDateTime( siteMatcher.group( 1 ) ).toInstant() ); if (clearContent) { site.setSitePassword( site.getResultType(), siteMatcher.group( 8 ) ); site.setLoginName( MPResultType.StoredPersonal, siteMatcher.group( 6 ) ); diff --git a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPJSONFile.java b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPJSONFile.java index 31509438..65582d51 100644 --- a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPJSONFile.java +++ b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPJSONFile.java @@ -144,7 +144,7 @@ public class MPJSONFile extends MPJSONAnyObject { (user.default_type != null)? user.default_type: algorithm.mpw_default_result_type(), (user.last_used != null)? MPModelConstants.dateTimeFormatter.parseDateTime( user.last_used ): new Instant(), user.hide_passwords, export.redacted? MPMarshaller.ContentMode.PROTECTED: MPMarshaller.ContentMode.VISIBLE, - MPMarshalFormat.JSON, file.getParentFile() + MPMarshalFormat.JSON, file ); } diff --git a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPMarshalFormat.java b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPMarshalFormat.java index 6a1a7381..e0849b51 100644 --- a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPMarshalFormat.java +++ b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPMarshalFormat.java @@ -18,6 +18,9 @@ package com.lyndir.masterpassword.model.impl; +import java.io.File; + + /** * @author lhunath, 2017-09-20 * @@ -72,4 +75,8 @@ public enum MPMarshalFormat { @SuppressWarnings("MethodReturnAlwaysConstant") public abstract String fileSuffix(); + + public boolean matches(final File file) { + return file.getName().endsWith( fileSuffix() ); + } } diff --git a/public/site b/public/site index d8d510b6..ebd2c741 160000 --- a/public/site +++ b/public/site @@ -1 +1 @@ -Subproject commit d8d510b6be2e2136a040624b8d0ed7590b6e7530 +Subproject commit ebd2c741fd11171eefa081792f9b14d6b2bac356