refactor: add missing docs and add explicit `NoKeysProvidedException`

4 files changed, 46 insertions, 11 deletions

diff --git a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt
index d99213d5..96b9cc88 100644
--- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt
+++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/GpgIdentifier.kt
@@ -8,7 +8,10 @@ package app.passwordstore.crypto
 import java.util.Locale
 import java.util.regex.Pattern
 
+/** Supertype for valid identifiers of GPG keys. */
 public sealed class GpgIdentifier {
+
+  /** A [GpgIdentifier] that represents either a long key ID or a fingerprint. */
   public data class KeyId(val id: Long) : GpgIdentifier() {
     override fun toString(): String {
       return convertKeyIdToHex(id)
@@ -32,6 +35,10 @@ public sealed class GpgIdentifier {
     }
   }
 
+  /**
+   * A [GpgIdentifier] that represents the textual name/email combination corresponding to a key.
+   * Despite the [email] property in this class, the value is not guaranteed to be a valid email.
+   */
   public data class UserId(val email: String) : GpgIdentifier() {
     override fun toString(): String {
       return email
@@ -45,6 +52,9 @@ public sealed class GpgIdentifier {
     private const val HEX_32_STRING_LENGTH = 8
     private const val TRUNCATED_FINGERPRINT_LENGTH = 16
 
+    /**
+     * Attempts to parse an untyped String identifier into a concrete subtype of [GpgIdentifier].
+     */
     @Suppress("ReturnCount")
     public fun fromString(identifier: String): GpgIdentifier? {
       if (identifier.isEmpty()) return null
diff --git a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt
index 47c06c4f..da77bf33 100644
--- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt
+++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/KeyUtils.kt
@@ -28,6 +28,10 @@ public object KeyUtils {
     return KeyId(keyRing.publicKey.keyID)
   }
 
+  /**
+   * Attempts to parse the given [PGPKey] into a [PGPKeyRing] and obtains the [UserId] of the
+   * corresponding public key.
+   */
   public fun tryGetEmail(key: PGPKey): UserId? {
     val keyRing = tryParseKeyring(key) ?: return null
     return UserId(keyRing.publicKey.userIDs.next())
diff --git a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt
index be2ec474..f6ef50b3 100644
--- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt
+++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPKeyManager.kt
@@ -36,6 +36,7 @@ constructor(
 
   private val keyDir = File(filesDir, KEY_DIR_NAME)
 
+  /** @see KeyManager.addKey */
   override suspend fun addKey(key: PGPKey, replace: Boolean): Result<PGPKey, Throwable> =
     withContext(dispatcher) {
       runSuspendCatching {
@@ -72,6 +73,7 @@ constructor(
       }
     }
 
+  /** @see KeyManager.removeKey */
   override suspend fun removeKey(identifier: GpgIdentifier): Result<Unit, Throwable> =
     withContext(dispatcher) {
       runSuspendCatching {
@@ -84,6 +86,7 @@ constructor(
       }
     }
 
+  /** @see KeyManager.getKeyById */
   override suspend fun getKeyById(id: GpgIdentifier): Result<PGPKey, Throwable> =
     withContext(dispatcher) {
       runSuspendCatching {
@@ -119,6 +122,7 @@ constructor(
       }
     }
 
+  /** @see KeyManager.getAllKeys */
   override suspend fun getAllKeys(): Result<List<PGPKey>, Throwable> =
     withContext(dispatcher) {
       runSuspendCatching {
@@ -129,14 +133,9 @@ constructor(
       }
     }
 
+  /** @see KeyManager.getKeyById */
   override suspend fun getKeyId(key: PGPKey): GpgIdentifier? = tryGetId(key)
 
-  // TODO: This is a temp hack for now and in future it should check that the GPGKeyManager can
-  // decrypt the file.
-  override fun canHandle(fileName: String): Boolean {
-    return fileName.endsWith(KEY_EXTENSION)
-  }
-
   /** Checks if [keyDir] exists and attempts to create it if not. */
   private fun keyDirExists(): Boolean {
     return keyDir.exists() || keyDir.mkdirs()
diff --git a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt
index 2bf68401..cf29931b 100644
--- a/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt
+++ b/crypto-pgpainless/src/main/kotlin/app/passwordstore/crypto/PGPainlessCryptoHandler.kt
@@ -7,6 +7,7 @@ package app.passwordstore.crypto
 
 import app.passwordstore.crypto.errors.CryptoHandlerException
 import app.passwordstore.crypto.errors.IncorrectPassphraseException
+import app.passwordstore.crypto.errors.NoKeysProvidedException
 import app.passwordstore.crypto.errors.UnknownError
 import com.github.michaelbull.result.Result
 import com.github.michaelbull.result.mapError
@@ -29,6 +30,13 @@ import org.pgpainless.util.Passphrase
 public class PGPainlessCryptoHandler @Inject constructor() :
   CryptoHandler<PGPKey, PGPEncryptOptions, PGPDecryptOptions> {
 
+  /**
+   * Decrypts the given [ciphertextStream] using [PGPainless] and writes the decrypted output to
+   * [outputStream]. The provided [passphrase] is wrapped in a [SecretKeyRingProtector] and the
+   * [keys] argument is defensively checked to ensure it has at least one key present.
+   *
+   * @see CryptoHandler.decrypt
+   */
   public override fun decrypt(
     keys: List<PGPKey>,
     passphrase: String,
@@ -37,7 +45,9 @@ public class PGPainlessCryptoHandler @Inject constructor() :
     options: PGPDecryptOptions,
   ): Result<Unit, CryptoHandlerException> =
     runCatching {
-        require(keys.isNotEmpty())
+        if (keys.isEmpty()) {
+          throw NoKeysProvidedException
+        }
         val keyringCollection =
           keys
             .map { key -> PGPainless.readKeyRing().secretKeyRing(key.contents) }
@@ -56,10 +66,17 @@ public class PGPainlessCryptoHandler @Inject constructor() :
       .mapError { error ->
         when (error) {
           is WrongPassphraseException -> IncorrectPassphraseException(error)
+          is CryptoHandlerException -> error
           else -> UnknownError(error)
         }
       }
 
+  /**
+   * Encrypts the provided [plaintextStream] and writes the encrypted output to [outputStream]. The
+   * [keys] argument is defensively checked to contain at least one key.
+   *
+   * @see CryptoHandler.encrypt
+   */
   public override fun encrypt(
     keys: List<PGPKey>,
     plaintextStream: InputStream,
@@ -67,7 +84,9 @@ public class PGPainlessCryptoHandler @Inject constructor() :
     options: PGPEncryptOptions,
   ): Result<Unit, CryptoHandlerException> =
     runCatching {
-        require(keys.isNotEmpty())
+        if (keys.isEmpty()) {
+          throw NoKeysProvidedException
+        }
         val publicKeyRings =
           keys.mapNotNull(KeyUtils::tryParseKeyring).mapNotNull { keyRing ->
             when (keyRing) {
@@ -77,9 +96,11 @@ public class PGPainlessCryptoHandler @Inject constructor() :
             }
           }
         require(keys.size == publicKeyRings.size) {
-          "Failed to parse all keys: keys=${keys.size},parsed=${publicKeyRings.size}"
+          "Failed to parse all keys: ${keys.size} keys were provided but only ${publicKeyRings.size} were valid"
+        }
+        if (publicKeyRings.isEmpty()) {
+          throw NoKeysProvidedException
         }
-        require(publicKeyRings.isNotEmpty()) { "No public keys to encrypt message to" }
         val publicKeyRingCollection = PGPPublicKeyRingCollection(publicKeyRings)
         val encryptionOptions = EncryptionOptions().addRecipients(publicKeyRingCollection)
         val producerOptions =
@@ -103,7 +124,8 @@ public class PGPainlessCryptoHandler @Inject constructor() :
         }
       }
 
+  /** Runs a naive check on the extension for the given [fileName] to check if it is a PGP file. */
   public override fun canHandle(fileName: String): Boolean {
-    return fileName.split('.').lastOrNull() == "gpg"
+    return fileName.substringAfterLast('.', "") == "gpg"
   }
 }