Reorganize crypto-common code and fix a couple minor bugs (#1868)

This commit is contained in:
Harsh Shandilya 2022-04-24 21:25:34 +05:30 committed by GitHub
parent 599abd37e8
commit 62902ca80b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 60 additions and 46 deletions

View file

@ -48,6 +48,8 @@ class PGPKeyImportActivity : AppCompatActivity() {
.setPositiveButton(android.R.string.ok) { _, _ -> finish() } .setPositiveButton(android.R.string.ok) { _, _ -> finish() }
.setOnCancelListener { finish() } .setOnCancelListener { finish() }
.show() .show()
} else {
finish()
} }
}, },
{ throwable -> { throwable ->

View file

@ -10,8 +10,10 @@ plugins { id("com.github.android-password-store.kotlin-common") }
tasks.withType<KotlinCompile>().configureEach { tasks.withType<KotlinCompile>().configureEach {
kotlinOptions { kotlinOptions {
if (project.providers.gradleProperty("android.injected.invoked.from.ide").orNull != "true") {
if (!name.contains("test", ignoreCase = true)) { if (!name.contains("test", ignoreCase = true)) {
freeCompilerArgs += listOf("-Xexplicit-api=strict") freeCompilerArgs = freeCompilerArgs + listOf("-Xexplicit-api=strict")
}
} }
} }
} }

View file

@ -1,29 +0,0 @@
package dev.msfjarvis.aps.crypto
public sealed class CryptoException(message: String? = null) : Exception(message)
/** Sealed exception types for [KeyManager]. */
public sealed class KeyManagerException(message: String? = null) : CryptoException(message) {
/** Store contains no keys. */
public object NoKeysAvailableException : KeyManagerException("No keys were found")
/** Key directory does not exist or cannot be accessed. */
public object KeyDirectoryUnavailableException :
KeyManagerException("Key directory does not exist")
/** Failed to delete given key. */
public object KeyDeletionFailedException : KeyManagerException("Couldn't delete the key file")
/** Failed to parse the key as a known type. */
public object InvalidKeyException :
KeyManagerException("Given key cannot be parsed as a known key type")
/** No key matching [keyId] could be found. */
public class KeyNotFoundException(keyId: String) :
KeyManagerException("No key found with id: $keyId")
/** Attempting to add another key for [keyId] without requesting a replace. */
public class KeyAlreadyExistsException(keyId: String) :
KeyManagerException("Pre-existing key was found for $keyId")
}

View file

@ -16,7 +16,7 @@ public interface KeyManager<Key, KeyIdentifier> {
/** /**
* Inserts a [key] into the store. If the key already exists, this method will return * Inserts a [key] into the store. If the key already exists, this method will return
* [KeyManagerException.KeyAlreadyExistsException] unless [replace] is `true`. * [KeyAlreadyExistsException] unless [replace] is `true`.
*/ */
public suspend fun addKey(key: Key, replace: Boolean = false): Result<Key, Throwable> public suspend fun addKey(key: Key, replace: Boolean = false): Result<Key, Throwable>

View file

@ -0,0 +1,30 @@
package dev.msfjarvis.aps.crypto.errors
import dev.msfjarvis.aps.crypto.KeyManager
public sealed class CryptoException(message: String? = null) : Exception(message)
/** Sealed exception types for [KeyManager]. */
public sealed class KeyManagerException(message: String? = null) : CryptoException(message)
/** Store contains no keys. */
public object NoKeysAvailableException : KeyManagerException("No keys were found")
/** Key directory does not exist or cannot be accessed. */
public object KeyDirectoryUnavailableException :
KeyManagerException("Key directory does not exist")
/** Failed to delete given key. */
public object KeyDeletionFailedException : KeyManagerException("Couldn't delete the key file")
/** Failed to parse the key as a known type. */
public object InvalidKeyException :
KeyManagerException("Given key cannot be parsed as a known key type")
/** No key matching [keyId] could be found. */
public class KeyNotFoundException(keyId: String) :
KeyManagerException("No key found with id: $keyId")
/** Attempting to add another key for [keyId] without requesting a replace. */
public class KeyAlreadyExistsException(keyId: String) :
KeyManagerException("Pre-existing key was found for $keyId")

View file

@ -10,6 +10,12 @@ import androidx.annotation.VisibleForTesting
import com.github.michaelbull.result.Result import com.github.michaelbull.result.Result
import dev.msfjarvis.aps.crypto.KeyUtils.tryGetId import dev.msfjarvis.aps.crypto.KeyUtils.tryGetId
import dev.msfjarvis.aps.crypto.KeyUtils.tryParseKeyring import dev.msfjarvis.aps.crypto.KeyUtils.tryParseKeyring
import dev.msfjarvis.aps.crypto.errors.InvalidKeyException
import dev.msfjarvis.aps.crypto.errors.KeyAlreadyExistsException
import dev.msfjarvis.aps.crypto.errors.KeyDeletionFailedException
import dev.msfjarvis.aps.crypto.errors.KeyDirectoryUnavailableException
import dev.msfjarvis.aps.crypto.errors.KeyNotFoundException
import dev.msfjarvis.aps.crypto.errors.NoKeysAvailableException
import dev.msfjarvis.aps.util.coroutines.runSuspendCatching import dev.msfjarvis.aps.util.coroutines.runSuspendCatching
import java.io.File import java.io.File
import javax.inject.Inject import javax.inject.Inject
@ -29,16 +35,16 @@ constructor(
override suspend fun addKey(key: PGPKey, replace: Boolean): Result<PGPKey, Throwable> = override suspend fun addKey(key: PGPKey, replace: Boolean): Result<PGPKey, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException if (!keyDirExists()) throw KeyDirectoryUnavailableException
if (tryParseKeyring(key) == null) throw KeyManagerException.InvalidKeyException if (tryParseKeyring(key) == null) throw InvalidKeyException
val keyFile = File(keyDir, "${tryGetId(key)}.$KEY_EXTENSION") val keyFile = File(keyDir, "${tryGetId(key)}.$KEY_EXTENSION")
if (keyFile.exists()) { if (keyFile.exists()) {
// Check for replace flag first and if it is false, throw an error // Check for replace flag first and if it is false, throw an error
if (!replace) if (!replace)
throw KeyManagerException.KeyAlreadyExistsException( throw KeyAlreadyExistsException(
tryGetId(key)?.toString() ?: "Failed to retrieve key ID" tryGetId(key)?.toString() ?: "Failed to retrieve key ID"
) )
if (!keyFile.delete()) throw KeyManagerException.KeyDeletionFailedException if (!keyFile.delete()) throw KeyDeletionFailedException
} }
keyFile.writeBytes(key.contents) keyFile.writeBytes(key.contents)
@ -50,11 +56,11 @@ constructor(
override suspend fun removeKey(key: PGPKey): Result<PGPKey, Throwable> = override suspend fun removeKey(key: PGPKey): Result<PGPKey, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException if (!keyDirExists()) throw KeyDirectoryUnavailableException
if (tryParseKeyring(key) == null) throw KeyManagerException.InvalidKeyException if (tryParseKeyring(key) == null) throw InvalidKeyException
val keyFile = File(keyDir, "${tryGetId(key)}.$KEY_EXTENSION") val keyFile = File(keyDir, "${tryGetId(key)}.$KEY_EXTENSION")
if (keyFile.exists()) { if (keyFile.exists()) {
if (!keyFile.delete()) throw KeyManagerException.KeyDeletionFailedException if (!keyFile.delete()) throw KeyDeletionFailedException
} }
key key
@ -64,9 +70,9 @@ constructor(
override suspend fun getKeyById(id: GpgIdentifier): Result<PGPKey, Throwable> = override suspend fun getKeyById(id: GpgIdentifier): Result<PGPKey, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException if (!keyDirExists()) throw KeyDirectoryUnavailableException
val keyFiles = keyDir.listFiles() val keyFiles = keyDir.listFiles()
if (keyFiles.isNullOrEmpty()) throw KeyManagerException.NoKeysAvailableException if (keyFiles.isNullOrEmpty()) throw NoKeysAvailableException
val keys = keyFiles.map { file -> PGPKey(file.readBytes()) } val keys = keyFiles.map { file -> PGPKey(file.readBytes()) }
val matchResult = val matchResult =
@ -92,14 +98,14 @@ constructor(
return@runSuspendCatching matchResult return@runSuspendCatching matchResult
} }
throw KeyManagerException.KeyNotFoundException("$id") throw KeyNotFoundException("$id")
} }
} }
override suspend fun getAllKeys(): Result<List<PGPKey>, Throwable> = override suspend fun getAllKeys(): Result<List<PGPKey>, Throwable> =
withContext(dispatcher) { withContext(dispatcher) {
runSuspendCatching { runSuspendCatching {
if (!keyDirExists()) throw KeyManagerException.KeyDirectoryUnavailableException if (!keyDirExists()) throw KeyDirectoryUnavailableException
val keyFiles = keyDir.listFiles() val keyFiles = keyDir.listFiles()
if (keyFiles.isNullOrEmpty()) return@runSuspendCatching emptyList() if (keyFiles.isNullOrEmpty()) return@runSuspendCatching emptyList()
keyFiles.map { keyFile -> PGPKey(keyFile.readBytes()) }.toList() keyFiles.map { keyFile -> PGPKey(keyFile.readBytes()) }.toList()

View file

@ -5,6 +5,9 @@ import com.github.michaelbull.result.unwrapError
import dev.msfjarvis.aps.crypto.GpgIdentifier.KeyId import dev.msfjarvis.aps.crypto.GpgIdentifier.KeyId
import dev.msfjarvis.aps.crypto.GpgIdentifier.UserId import dev.msfjarvis.aps.crypto.GpgIdentifier.UserId
import dev.msfjarvis.aps.crypto.TestUtils.getArmoredPrivateKeyWithMultipleIdentities import dev.msfjarvis.aps.crypto.TestUtils.getArmoredPrivateKeyWithMultipleIdentities
import dev.msfjarvis.aps.crypto.errors.KeyAlreadyExistsException
import dev.msfjarvis.aps.crypto.errors.KeyNotFoundException
import dev.msfjarvis.aps.crypto.errors.NoKeysAvailableException
import java.io.File import java.io.File
import kotlin.test.AfterTest import kotlin.test.AfterTest
import kotlin.test.BeforeTest import kotlin.test.BeforeTest
@ -69,7 +72,7 @@ class PGPKeyManagerTest {
keyManager.addKey(key, false).unwrap() keyManager.addKey(key, false).unwrap()
val error = keyManager.addKey(key, false).unwrapError() val error = keyManager.addKey(key, false).unwrapError()
assertIs<KeyManagerException.KeyAlreadyExistsException>(error) assertIs<KeyAlreadyExistsException>(error)
} }
@Test @Test
@ -142,7 +145,7 @@ class PGPKeyManagerTest {
// Check returned key // Check returned key
val error = keyManager.getKeyById(keyId).unwrapError() val error = keyManager.getKeyById(keyId).unwrapError()
assertIs<KeyManagerException.KeyNotFoundException>(error) assertIs<KeyNotFoundException>(error)
assertEquals("No key found with id: $keyId", error.message) assertEquals("No key found with id: $keyId", error.message)
} }
@ -151,7 +154,7 @@ class PGPKeyManagerTest {
scope.runTest { scope.runTest {
// Check returned key // Check returned key
val error = keyManager.getKeyById(KeyId(0x08edf7567183ce44)).unwrapError() val error = keyManager.getKeyById(KeyId(0x08edf7567183ce44)).unwrapError()
assertIs<KeyManagerException.NoKeysAvailableException>(error) assertIs<NoKeysAvailableException>(error)
assertEquals("No keys were found", error.message) assertEquals("No keys were found", error.message)
} }