Merge branch '1341-account-manager-refactoring' into 'master'

Refactor authentication and account management code Closes #1341 See merge request briar/briar!866
2026-02-14 19:59:05 +01:00 · 2018-08-02 11:49:15 +00:00
parent 55d8f853db c1789a8e52
commit c0d5a3a5ea
65 changed files with 1333 additions and 1014 deletions
--- a/bramble-android/build.gradle
+++ b/bramble-android/build.gradle
@@ -34,6 +34,14 @@ dependencies {

 	compileOnly 'javax.annotation:jsr250-api:1.0'

+	testImplementation project(path: ':bramble-api', configuration: 'testOutput')
+	testImplementation 'junit:junit:4.12'
+	testImplementation "org.jmock:jmock:2.8.2"
+	testImplementation "org.jmock:jmock-junit4:2.8.2"
+	testImplementation "org.jmock:jmock-legacy:2.8.2"
+	testImplementation "org.hamcrest:hamcrest-library:1.3"
+	testImplementation "org.hamcrest:hamcrest-core:1.3"
+
 	androidTestImplementation project(path: ':bramble-api', configuration: 'testOutput')
 	androidTestImplementation project(path: ':bramble-core', configuration: 'testOutput')
 	androidTestImplementation 'com.android.support.test:runner:1.0.2'
@@ -43,6 +51,10 @@ dependencies {

 dependencyVerification {
 	verify = [
+			'cglib:cglib:3.2.0:cglib-3.2.0.jar:adb13bab79712ad6bdf1bd59f2a3918018a8016e722e8a357065afb9e6690861',
+			'com.android.support.test:monitor:1.0.2:monitor-1.0.2.aar:38ef4fa98a32dc55550ff49bb36a583e178b3a9b830fcb8dcc27bfc4254bc2bc',
+			'com.android.support.test:runner:1.0.2:runner-1.0.2.aar:f04b9ae342975ba1cb3e4a06e13426e3e6b8a73faa45acba604493d83c9a4f00',
+			'com.android.support:support-annotations:27.1.1:support-annotations-27.1.1.jar:3365960206c3d2b09e845f555e7f88f8effc8d2f00b369e66c4be384029299cf',
 			'com.android.tools.analytics-library:protos:26.1.3:protos-26.1.3.jar:818c9f256f141d9dafec03a1aa2b94d240b2c140acfd7ee31a8b3e6c2b9479e3',
 			'com.android.tools.analytics-library:shared:26.1.3:shared-26.1.3.jar:7110706c7ada96c8b6f5ca80c478291bc7899d46277de2c48527e045442401a3',
 			'com.android.tools.analytics-library:tracker:26.1.3:tracker-26.1.3.jar:4155424bf2ce4872da83332579a1707252bc66cbd77c5144fdc4483d0f2e1418',
@@ -91,12 +103,16 @@ dependencyVerification {
 			'javax.annotation:jsr250-api:1.0:jsr250-api-1.0.jar:a1a922d0d9b6d183ed3800dfac01d1e1eb159f0e8c6f94736931c1def54a941f',
 			'javax.inject:javax.inject:1:javax.inject-1.jar:91c77044a50c481636c32d916fd89c9118a72195390452c81065080f957de7ff',
 			'javax.xml.bind:jaxb-api:2.2.12-b140109.1041:jaxb-api-2.2.12-b140109.1041.jar:b5e60cd8b7b5ff01ce4a74c5dd008f4fbd14ced3495d0b47b85cfedc182211f2',
+			'junit:junit:4.12:junit-4.12.jar:59721f0805e223d84b90677887d9ff567dc534d7c502ca903c0c2b17f05c116a',
 			'net.sf.jopt-simple:jopt-simple:4.9:jopt-simple-4.9.jar:26c5856e954b5f864db76f13b86919b59c6eecf9fd930b96baa8884626baf2f5',
 			'net.sf.kxml:kxml2:2.3.0:kxml2-2.3.0.jar:f264dd9f79a1fde10ce5ecc53221eff24be4c9331c830b7d52f2f08a7b633de2',
+			'org.apache.ant:ant-launcher:1.9.4:ant-launcher-1.9.4.jar:7bccea20b41801ca17bcbc909a78c835d0f443f12d639c77bd6ae3d05861608d',
+			'org.apache.ant:ant:1.9.4:ant-1.9.4.jar:649ae0730251de07b8913f49286d46bba7b92d47c5f332610aa426c4f02161d8',
 			'org.apache.commons:commons-compress:1.12:commons-compress-1.12.jar:2c1542faf343185b7cab9c3d55c8ae5471d6d095d3887a4adefdbdf2984dc0b6',
 			'org.apache.httpcomponents:httpclient:4.2.6:httpclient-4.2.6.jar:362e9324ee7c697e21279e20077b52737ddef3f1b2c1a7abe5ad34b465145550',
 			'org.apache.httpcomponents:httpcore:4.2.5:httpcore-4.2.5.jar:e5e82da4cc66c8d917bbf743e3c0752efe8522735e7fc9dbddb65bccea81cfe9',
 			'org.apache.httpcomponents:httpmime:4.1:httpmime-4.1.jar:31629566148e8a47688ae43b420abc3ecd783ed15b33bebc00824bf24c9b15aa',
+			'org.beanshell:bsh:1.3.0:bsh-1.3.0.jar:9b04edc75d19db54f1b4e8b5355e9364384c6cf71eb0a1b9724c159d779879f8',
 			'org.bouncycastle:bcpkix-jdk15on:1.56:bcpkix-jdk15on-1.56.jar:7043dee4e9e7175e93e0b36f45b1ec1ecb893c5f755667e8b916eb8dd201c6ca',
 			'org.bouncycastle:bcprov-jdk15on:1.56:bcprov-jdk15on-1.56.jar:963e1ee14f808ffb99897d848ddcdb28fa91ddda867eb18d303e82728f878349',
 			'org.briarproject:jtorctl:0.3:jtorctl-0.3.jar:f2939238a097898998432effe93b0334d97a787972ab3a91a8973a1d309fc864',
@@ -106,17 +122,25 @@ dependencyVerification {
 			'org.glassfish.jaxb:jaxb-core:2.2.11:jaxb-core-2.2.11.jar:37bcaee8ebb04362c8352a5bf6221b86967ecdab5164c696b10b9a2bb587b2aa',
 			'org.glassfish.jaxb:jaxb-runtime:2.2.11:jaxb-runtime-2.2.11.jar:a874f2351cfba8e2946be3002d10c18a6da8f21b52ba2acf52f2b85d5520ed70',
 			'org.glassfish.jaxb:txw2:2.2.11:txw2-2.2.11.jar:272a3ccad45a4511351920cd2a8633c53cab8d5220c7a92954da5526bb5eafea',
+			'org.hamcrest:hamcrest-core:1.3:hamcrest-core-1.3.jar:66fdef91e9739348df7a096aa384a5685f4e875584cce89386a7a47251c4d8e9',
+			'org.hamcrest:hamcrest-library:1.3:hamcrest-library-1.3.jar:711d64522f9ec410983bd310934296da134be4254a125080a0416ec178dfad1c',
 			'org.jetbrains.kotlin:kotlin-reflect:1.2.0:kotlin-reflect-1.2.0.jar:4f48a872bad6e4d9c053f4ad610d11e4012ad7e58dc19a03dd5eb811f36069dd',
 			'org.jetbrains.kotlin:kotlin-stdlib-jre7:1.2.0:kotlin-stdlib-jre7-1.2.0.jar:c7a20fb951d437797afe8980aff6c1e5a03f310c661ba58ba1d4fa90cb0f2926',
 			'org.jetbrains.kotlin:kotlin-stdlib-jre8:1.2.0:kotlin-stdlib-jre8-1.2.0.jar:633524eee6ef1941f7cb1dab7ee3927b0a221ceee9047aeb5515f4cbb990c82a',
 			'org.jetbrains.kotlin:kotlin-stdlib:1.2.0:kotlin-stdlib-1.2.0.jar:05cfd9f5ac0b41910703a8925f7211a495909b27a2ffdd1c5106f1689aeafcd4',
 			'org.jetbrains.trove4j:trove4j:20160824:trove4j-20160824.jar:1917871c8deb468307a584680c87a44572f5a8b0b98c6d397fc0f5f86596dbe7',
 			'org.jetbrains:annotations:13.0:annotations-13.0.jar:ace2a10dc8e2d5fd34925ecac03e4988b2c0f851650c94b8cef49ba1bd111478',
+			'org.jmock:jmock-junit4:2.8.2:jmock-junit4-2.8.2.jar:f7ee4df4f7bd7b7f1cafad3b99eb74d579f109d5992ff625347352edb55e674c',
+			'org.jmock:jmock-legacy:2.8.2:jmock-legacy-2.8.2.jar:f2b985a5c08a9edb7f37612330c058809da3f6a6d63ce792426ebf8ff0d6d31b',
+			'org.jmock:jmock-testjar:2.8.2:jmock-testjar-2.8.2.jar:8900860f72c474e027cf97fe78dcbf154a1aa7fc62b6845c5fb4e4f3c7bc8760',
+			'org.jmock:jmock:2.8.2:jmock-2.8.2.jar:6c73cb4a2e6dbfb61fd99c9a768539c170ab6568e57846bd60dbf19596b65b16',
 			'org.jvnet.staxex:stax-ex:1.7.7:stax-ex-1.7.7.jar:a31ff7d77163c0deb09e7fee59ad35ae44c2cee2cc8552a116ccd1583d813fb4',
+			'org.objenesis:objenesis:2.1:objenesis-2.1.jar:c74330cc6b806c804fd37e74487b4fe5d7c2750c5e15fbc6efa13bdee1bdef80',
 			'org.ow2.asm:asm-analysis:5.1:asm-analysis-5.1.jar:a34658f5c5de4b573eef21131cc32cc25f7b66407944f312b28ec2e56abb1fa9',
 			'org.ow2.asm:asm-commons:5.1:asm-commons-5.1.jar:97b3786e1f55e74bddf8ad102bf50e33bbcbc1f6b7fd7b36f0bbbb25cd4981be',
 			'org.ow2.asm:asm-tree:5.1:asm-tree-5.1.jar:c0de2bbc4cb8297419659813ecd4ed1d077ed1dd5c1f5544cc5143e493e84c10',
 			'org.ow2.asm:asm-util:5.1:asm-util-5.1.jar:ee032c39ae5e3cd099148fbba9a2124f9ed613e5cb93e03ee0fa8808ce364040',
+			'org.ow2.asm:asm:5.0.4:asm-5.0.4.jar:896618ed8ae62702521a78bc7be42b7c491a08e6920a15f89a3ecdec31e9a220',
 			'org.ow2.asm:asm:5.1:asm-5.1.jar:d2da399a9967c69f0a21739256fa79d284222c223082cacadc17372244764b54',
 	]
 }
--- a/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java
+++ b/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java
@@ -0,0 +1,108 @@
+package org.briarproject.bramble.account;
+
+import android.app.Application;
+import android.content.Context;
+import android.content.SharedPreferences;
+import android.preference.PreferenceManager;
+
+import org.briarproject.bramble.api.account.AccountManager;
+import org.briarproject.bramble.api.crypto.CryptoComponent;
+import org.briarproject.bramble.api.db.DatabaseConfig;
+import org.briarproject.bramble.api.identity.IdentityManager;
+import org.briarproject.bramble.util.IoUtils;
+
+import java.io.File;
+import java.util.logging.Logger;
+
+import javax.annotation.Nullable;
+import javax.inject.Inject;
+
+class AndroidAccountManager extends AccountManagerImpl
+		implements AccountManager {
+
+	private static final Logger LOG =
+			Logger.getLogger(AndroidAccountManager.class.getName());
+
+	private static final String PREF_DB_KEY = "key";
+
+	protected final Context appContext;
+	private final SharedPreferences prefs;
+
+	@Inject
+	AndroidAccountManager(DatabaseConfig databaseConfig,
+			CryptoComponent crypto, IdentityManager identityManager,
+			SharedPreferences prefs, Application app) {
+		super(databaseConfig, crypto, identityManager);
+		this.prefs = prefs;
+		appContext = app.getApplicationContext();
+	}
+
+	// Locking: stateChangeLock
+	@Override
+	@Nullable
+	protected String loadEncryptedDatabaseKey() {
+		String key = getDatabaseKeyFromPreferences();
+		if (key == null) key = super.loadEncryptedDatabaseKey();
+		else migrateDatabaseKeyToFile(key);
+		return key;
+	}
+
+	// Locking: stateChangeLock
+	@Nullable
+	private String getDatabaseKeyFromPreferences() {
+		String key = prefs.getString(PREF_DB_KEY, null);
+		if (key == null) LOG.info("No database key in preferences");
+		else LOG.info("Found database key in preferences");
+		return key;
+	}
+
+	// Locking: stateChangeLock
+	private void migrateDatabaseKeyToFile(String key) {
+		if (storeEncryptedDatabaseKey(key)) {
+			if (prefs.edit().remove(PREF_DB_KEY).commit())
+				LOG.info("Database key migrated to file");
+			else LOG.warning("Database key not removed from preferences");
+		} else {
+			LOG.warning("Database key not migrated to file");
+		}
+	}
+
+	@Override
+	public void deleteAccount() {
+		synchronized (stateChangeLock) {
+			super.deleteAccount();
+			SharedPreferences defaultPrefs = getDefaultSharedPreferences();
+			deleteAppData(prefs, defaultPrefs);
+		}
+	}
+
+	// Package access for testing
+	SharedPreferences getDefaultSharedPreferences() {
+		return PreferenceManager.getDefaultSharedPreferences(appContext);
+	}
+
+	// Locking: stateChangeLock
+	private void deleteAppData(SharedPreferences... clear) {
+		// Clear and commit shared preferences
+		for (SharedPreferences prefs : clear) {
+			if (!prefs.edit().clear().commit())
+				LOG.warning("Could not clear shared preferences");
+		}
+		// Delete files, except lib and shared_prefs directories
+		File dataDir = new File(appContext.getApplicationInfo().dataDir);
+		File[] children = dataDir.listFiles();
+		if (children == null) {
+			LOG.warning("Could not list files in app data dir");
+		} else {
+			for (File child : children) {
+				String name = child.getName();
+				if (!name.equals("lib") && !name.equals("shared_prefs")) {
+					IoUtils.deleteFileOrDir(child);
+				}
+			}
+		}
+		// Recreate the cache dir as some OpenGL drivers expect it to exist
+		if (!new File(dataDir, "cache").mkdir())
+			LOG.warning("Could not recreate cache dir");
+	}
+}
--- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/CircumventionProvider.java
+++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/CircumventionProvider.java
@@ -4,6 +4,8 @@ import org.briarproject.bramble.api.lifecycle.IoExecutor;

 import java.util.List;

+// TODO: Create a module for this so it doesn't have to be public
+
 public interface CircumventionProvider {

 	/**
--- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/CircumventionProviderImpl.java
+++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/CircumventionProviderImpl.java
@@ -16,6 +16,8 @@ import java.util.Set;
 import javax.annotation.Nullable;
 import javax.inject.Inject;

+// TODO: Create a module for this so it doesn't need to be public
+
 public class CircumventionProviderImpl implements CircumventionProvider {

 	private final static String BRIDGE_FILE_NAME = "bridges";
--- a/bramble-android/src/main/java/org/briarproject/bramble/util/AndroidUtils.java
+++ b/bramble-android/src/main/java/org/briarproject/bramble/util/AndroidUtils.java
@@ -3,7 +3,6 @@ package org.briarproject.bramble.util;
 import android.annotation.SuppressLint;
 import android.bluetooth.BluetoothAdapter;
 import android.content.Context;
-import android.content.SharedPreferences;
 import android.os.Build;
 import android.provider.Settings;

@@ -58,30 +57,6 @@ public class AndroidUtils {
 				&& !address.equals(FAKE_BLUETOOTH_ADDRESS);
 	}

-	public static void deleteAppData(Context ctx, SharedPreferences... clear) {
-		// Clear and commit shared preferences
-		for (SharedPreferences prefs : clear) {
-			if (!prefs.edit().clear().commit())
-				LOG.warning("Could not clear shared preferences");
-		}
-		// Delete files, except lib and shared_prefs directories
-		File dataDir = new File(ctx.getApplicationInfo().dataDir);
-		File[] children = dataDir.listFiles();
-		if (children == null) {
-			LOG.warning("Could not list files in app data dir");
-		} else {
-			for (File child : children) {
-				String name = child.getName();
-				if (!name.equals("lib") && !name.equals("shared_prefs")) {
-					IoUtils.deleteFileOrDir(child);
-				}
-			}
-		}
-		// Recreate the cache dir as some OpenGL drivers expect it to exist
-		if (!new File(dataDir, "cache").mkdir())
-			LOG.warning("Could not recreate cache dir");
-	}
-
 	public static File getReportDir(Context ctx) {
 		return ctx.getDir(STORED_REPORTS, MODE_PRIVATE);
 	}
--- a/bramble-android/src/test/java/org/briarproject/bramble/account/AndroidAccountManagerTest.java
+++ b/bramble-android/src/test/java/org/briarproject/bramble/account/AndroidAccountManagerTest.java
@@ -0,0 +1,162 @@
+package org.briarproject.bramble.account;
+
+import android.app.Application;
+import android.content.SharedPreferences;
+import android.content.pm.ApplicationInfo;
+
+import org.briarproject.bramble.api.crypto.CryptoComponent;
+import org.briarproject.bramble.api.db.DatabaseConfig;
+import org.briarproject.bramble.api.identity.IdentityManager;
+import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.jmock.Expectations;
+import org.jmock.lib.legacy.ClassImposteriser;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.File;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertTrue;
+import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory;
+import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
+import static org.briarproject.bramble.test.TestUtils.getTestDirectory;
+import static org.briarproject.bramble.util.StringUtils.toHexString;
+
+public class AndroidAccountManagerTest extends BrambleMockTestCase {
+
+	private final SharedPreferences prefs =
+			context.mock(SharedPreferences.class, "prefs");
+	private final SharedPreferences defaultPrefs =
+			context.mock(SharedPreferences.class, "defaultPrefs");
+	private final DatabaseConfig databaseConfig =
+			context.mock(DatabaseConfig.class);
+	private final CryptoComponent crypto = context.mock(CryptoComponent.class);
+	private final IdentityManager identityManager =
+			context.mock(IdentityManager.class);
+	private final SharedPreferences.Editor
+			editor = context.mock(SharedPreferences.Editor.class);
+	private final Application app;
+	private final ApplicationInfo applicationInfo;
+
+	private final String encryptedKeyHex = toHexString(getRandomBytes(123));
+	private final File testDir = getTestDirectory();
+	private final File keyDir = new File(testDir, "key");
+	private final File keyFile = new File(keyDir, "db.key");
+	private final File keyBackupFile = new File(keyDir, "db.key.bak");
+	private final File dbDir = new File(testDir, "db");
+
+	private AndroidAccountManager accountManager;
+
+	public AndroidAccountManagerTest() {
+		context.setImposteriser(ClassImposteriser.INSTANCE);
+		app = context.mock(Application.class);
+		applicationInfo = new ApplicationInfo();
+		applicationInfo.dataDir = testDir.getAbsolutePath();
+	}
+
+	@Before
+	public void setUp() {
+		context.checking(new Expectations() {{
+			allowing(databaseConfig).getDatabaseDirectory();
+			will(returnValue(dbDir));
+			allowing(databaseConfig).getDatabaseKeyDirectory();
+			will(returnValue(keyDir));
+			allowing(app).getApplicationContext();
+			will(returnValue(app));
+		}});
+		accountManager = new AndroidAccountManager(databaseConfig, crypto,
+				identityManager, prefs, app) {
+			@Override
+			SharedPreferences getDefaultSharedPreferences() {
+				return defaultPrefs;
+			}
+		};
+	}
+
+	@Test
+	public void testDbKeyIsMigratedFromPreferencesToFile() {
+		context.checking(new Expectations() {{
+			oneOf(prefs).getString("key", null);
+			will(returnValue(encryptedKeyHex));
+			oneOf(prefs).edit();
+			will(returnValue(editor));
+			oneOf(editor).remove("key");
+			will(returnValue(editor));
+			oneOf(editor).commit();
+			will(returnValue(true));
+		}});
+
+		assertFalse(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+
+		assertEquals(encryptedKeyHex,
+				accountManager.loadEncryptedDatabaseKey());
+
+		assertTrue(keyFile.exists());
+		assertTrue(keyBackupFile.exists());
+	}
+
+	@Test
+	public void testDeleteAccountClearsSharedPrefsAndDeletesFiles()
+			throws Exception {
+		// Directories 'lib' and 'shared_prefs' should be spared
+		File libDir = new File(testDir, "lib");
+		File libFile = new File(libDir, "file");
+		File sharedPrefsDir = new File(testDir, "shared_prefs");
+		File sharedPrefsFile = new File(sharedPrefsDir, "file");
+		// Directory 'cache' should be emptied
+		File cacheDir = new File(testDir, "cache");
+		File cacheFile = new File(cacheDir, "file");
+		// Other directories should be deleted
+		File potatoDir = new File(testDir, ".potato");
+		File potatoFile = new File(potatoDir, "file");
+
+		context.checking(new Expectations() {{
+			oneOf(prefs).edit();
+			will(returnValue(editor));
+			oneOf(editor).clear();
+			will(returnValue(editor));
+			oneOf(editor).commit();
+			will(returnValue(true));
+			oneOf(defaultPrefs).edit();
+			will(returnValue(editor));
+			oneOf(editor).clear();
+			will(returnValue(editor));
+			oneOf(editor).commit();
+			will(returnValue(true));
+			oneOf(app).getApplicationInfo();
+			will(returnValue(applicationInfo));
+		}});
+
+		assertTrue(dbDir.mkdirs());
+		assertTrue(keyDir.mkdirs());
+		assertTrue(libDir.mkdirs());
+		assertTrue(libFile.createNewFile());
+		assertTrue(sharedPrefsDir.mkdirs());
+		assertTrue(sharedPrefsFile.createNewFile());
+		assertTrue(cacheDir.mkdirs());
+		assertTrue(cacheFile.createNewFile());
+		assertTrue(potatoDir.mkdirs());
+		assertTrue(potatoFile.createNewFile());
+
+		accountManager.deleteAccount();
+
+		assertFalse(dbDir.exists());
+		assertFalse(keyDir.exists());
+		assertTrue(libDir.exists());
+		assertTrue(libFile.exists());
+		assertTrue(sharedPrefsDir.exists());
+		assertTrue(sharedPrefsFile.exists());
+		assertTrue(cacheDir.exists());
+		assertFalse(cacheFile.exists());
+		assertFalse(potatoDir.exists());
+		assertFalse(potatoFile.exists());
+	}
+
+	@After
+	public void tearDown() {
+		deleteTestDirectory(testDir);
+	}
+}