Support using Key.isPrivate() for type inference, remove Key.isPublic() (#1347)

API changes:
- `Key.isPublic()` has been removed, since it was redundant and it would
introduce TypeScript issues. Call `!Key.isPrivate()` instead.

TypeScript changes:
- the `openpgp.readKey(s)` functions are now declared as returning a `Key`
instead of a `PublicKey`. This is just a readability improvement to make it
clearer that the result could also be a `PrivateKey`.
- All `Key` methods that return a key object now have the narrowest possible
return type.
- The `Key.isPrivate()` method can now be used for type inference, allowing the
compiler to distinguish between `PrivateKey` and `PublicKey`. 

Calling `key.isPrivate()` is the recommended way of distinguishing between a
`PrivateKey` and `PublicKey` at runtime, over using `key instanceof ...`, since
the latter depends on the specifics of the `Key` class hierarchy.

openpgp.d.ts

@@ -8,11 +8,12 @@
  */
 
 /* ############## v5 KEY #################### */
-
+// The Key and PublicKey types can be used interchangably since TS cannot detect the difference, as they have the same class properties.
-export function readKey(options: { armoredKey: string, config?: PartialConfig }): Promise<PublicKey>;
+// The declared readKey(s) return type is Key instead of a PublicKey since it seems more obvious that a Key can be cast to a PrivateKey.
-export function readKey(options: { binaryKey: Uint8Array, config?: PartialConfig }): Promise<PublicKey>;
+export function readKey(options: { armoredKey: string, config?: PartialConfig }): Promise<Key>;
-export function readKeys(options: { armoredKeys: string, config?: PartialConfig }): Promise<PublicKey[]>;
+export function readKey(options: { binaryKey: Uint8Array, config?: PartialConfig }): Promise<Key>;
-export function readKeys(options: { binaryKeys: Uint8Array, config?: PartialConfig }): Promise<PublicKey[]>;
+export function readKeys(options: { armoredKeys: string, config?: PartialConfig }): Promise<Key[]>;
+export function readKeys(options: { binaryKeys: Uint8Array, config?: PartialConfig }): Promise<Key[]>;
 export function readPrivateKey(options: { armoredKey: string, config?: PartialConfig }): Promise<PrivateKey>;
 export function readPrivateKey(options: { binaryKey: Uint8Array, config?: PartialConfig }): Promise<PrivateKey>;
 export function readPrivateKeys(options: { armoredKeys: string, config?: PartialConfig }): Promise<PrivateKey[]>;
@@ -46,20 +47,21 @@ export abstract class Key {
   public getKeyIDs(): KeyID[];
   public getPrimaryUser(date?: Date, userID?: UserID, config?: Config): Promise<PrimaryUser>; // throws on error
   public getUserIDs(): string[];
-  public isPrivate(): boolean;
+  public isPrivate(): this is PrivateKey;
-  public isPublic(): boolean;
   public toPublic(): PublicKey;
+  // NB: the order of the `update` declarations matters, since PublicKey includes PrivateKey
+  public update(sourceKey: PrivateKey, date?: Date, config?: Config): Promise<PrivateKey>;
   public update(sourceKey: PublicKey, date?: Date, config?: Config): Promise<PublicKey>;
-  public signPrimaryUser(privateKeys: PrivateKey[], date?: Date, userID?: UserID, config?: Config): Promise<PublicKey>
+  public signPrimaryUser(privateKeys: PrivateKey[], date?: Date, userID?: UserID, config?: Config): Promise<this>
-  public signAllUsers(privateKeys: PrivateKey[], date?: Date, config?: Config): Promise<PublicKey>
+  public signAllUsers(privateKeys: PrivateKey[], date?: Date, config?: Config): Promise<this>
   public verifyPrimaryKey(date?: Date, userID?: UserID, config?: Config): Promise<void>; // throws on error
   public verifyPrimaryUser(publicKeys: PublicKey[], date?: Date, userIDs?: UserID, config?: Config): Promise<{ keyID: KeyID, valid: boolean | null }[]>;
   public verifyAllUsers(publicKeys: PublicKey[], date?: Date, config?: Config): Promise<{ userID: string, keyID: KeyID, valid: boolean | null }[]>;
   public isRevoked(signature: SignaturePacket, key?: AnyKeyPacket, date?: Date, config?: Config): Promise<boolean>;
   public getRevocationCertificate(date?: Date, config?: Config): Promise<Stream<string> | string | undefined>;
-  public getEncryptionKey(keyID?: KeyID, date?: Date | null, userID?: UserID, config?: Config): Promise<PublicKey | Subkey>;
+  public getEncryptionKey(keyID?: KeyID, date?: Date | null, userID?: UserID, config?: Config): Promise<this | Subkey>;
-  public getSigningKey(keyID?: KeyID, date?: Date | null, userID?: UserID, config?: Config): Promise<PublicKey | Subkey>;
+  public getSigningKey(keyID?: KeyID, date?: Date | null, userID?: UserID, config?: Config): Promise<this | Subkey>;
-  public getKeys(keyID?: KeyID): (PublicKey | Subkey)[];
+  public getKeys(keyID?: KeyID): (this | Subkey)[];
   public getSubkeys(keyID?: KeyID): Subkey[];
   public getFingerprint(): string;
   public getCreationTime(): Date;
@@ -80,7 +82,6 @@ export class PrivateKey extends PublicKey {
   public addSubkey(options: SubkeyOptions): Promise<PrivateKey>;
   public getDecryptionKeys(keyID?: KeyID, date?: Date | null, userID?: UserID, config?: Config): Promise<PrivateKey | Subkey>
   public update(sourceKey: PublicKey, date?: Date, config?: Config): Promise<PrivateKey>;
-  public getKeys(keyID?: KeyID): (PrivateKey | Subkey)[];
 }
 
 export class Subkey {

src/key/factory.js

@@ -95,7 +95,7 @@ export async function reformat(options, config) {
   options = sanitize(options);
   const { privateKey } = options;
 
-  if (privateKey.isPublic()) {
+  if (!privateKey.isPrivate()) {
     throw new Error('Cannot reformat a public key');
   }
 

src/key/key.js

@@ -476,7 +476,7 @@ class Key {
     if (!this.hasSameFingerprintAs(sourceKey)) {
       throw new Error('Primary key fingerprints must be equal to update the key');
     }
-    if (this.isPublic() && sourceKey.isPrivate()) {
+    if (!this.isPrivate() && sourceKey.isPrivate()) {
       // check for equal subkey packets
       const equal = (this.subkeys.length === sourceKey.subkeys.length) &&
             (this.subkeys.every(destSubkey => {

src/key/private_key.js

@@ -24,15 +24,6 @@ class PrivateKey extends PublicKey {
     }
   }
 
-  /**
-   * Returns true if this is a public key
-   * @returns {Boolean}
-   */
-  // eslint-disable-next-line class-methods-use-this
-  isPublic() {
-    return false;
-  }
-
   /**
    * Returns true if this is a private key
    * @returns {Boolean}
@@ -192,7 +183,7 @@ class PrivateKey extends PublicKey {
     date = new Date(),
     config = defaultConfig
   ) {
-    if (this.isPublic()) {
+    if (!this.isPrivate()) {
       throw new Error('Need private key for revoking');
     }
     const dataToSign = { key: this.keyPacket };

src/key/public_key.js

@@ -40,18 +40,9 @@ class PublicKey extends Key {
     }
   }
 
-  /**
-   * Returns true if this is a public key
-   * @returns {Boolean}
-   */
-  // eslint-disable-next-line class-methods-use-this
-  isPublic() {
-    return true;
-  }
-
   /**
    * Returns true if this is a private key
-   * @returns {Boolean}
+   * @returns {false}
    */
   // eslint-disable-next-line class-methods-use-this
   isPrivate() {

src/key/user.js

@@ -65,7 +65,7 @@ class User {
     };
     const user = new User(dataToSign.userID || dataToSign.userAttribute, this.mainKey);
     user.otherCertifications = await Promise.all(signingKeys.map(async function(privateKey) {
-      if (privateKey.isPublic()) {
+      if (!privateKey.isPrivate()) {
         throw new Error('Need private key for signing');
       }
       if (privateKey.hasSameFingerprintAs(primaryKey)) {

src/message.js

@@ -463,7 +463,7 @@ export class Message {
     }
 
     await Promise.all(Array.from(signingKeys).reverse().map(async function (primaryKey, i) {
-      if (primaryKey.isPublic()) {
+      if (!primaryKey.isPrivate()) {
         throw new Error('Need private key for signing');
       }
       const signingKeyID = signingKeyIDs[signingKeys.length - 1 - i];
@@ -672,7 +672,7 @@ export async function createSignaturePackets(literalDataPacket, signingKeys, sig
 
   await Promise.all(signingKeys.map(async (primaryKey, i) => {
     const userID = userIDs[i];
-    if (primaryKey.isPublic()) {
+    if (!primaryKey.isPrivate()) {
       throw new Error('Need private key for signing');
     }
     const signingKey = await primaryKey.getSigningKey(signingKeyIDs[i], date, userID, config);

test/general/key.js

@@ -3164,7 +3164,7 @@ module.exports = () => describe('Key', function() {
   it('update() - merge private key into public key', async function() {
     const source = await openpgp.readKey({ armoredKey: priv_key_rsa });
     const [dest] = await openpgp.readKeys({ armoredKeys: twoKeys });
-    expect(dest.isPublic()).to.be.true;
+    expect(dest.isPrivate()).to.be.false;
     return dest.update(source).then(async updated => {
       expect(updated.isPrivate()).to.be.true;
       return Promise.all([
@@ -3186,7 +3186,7 @@ module.exports = () => describe('Key', function() {
     const [dest] = await openpgp.readKeys({ armoredKeys: twoKeys });
     source.subkeys = [];
     dest.subkeys = [];
-    expect(dest.isPublic()).to.be.true;
+    expect(dest.isPrivate()).to.be.false;
 
     const updated = await dest.update(source);
     expect(updated.isPrivate()).to.be.true;
@@ -3203,7 +3203,7 @@ module.exports = () => describe('Key', function() {
     const [dest] = await openpgp.readKeys({ armoredKeys: twoKeys });
     source.subkeys = [];
     expect(dest.subkeys).to.exist;
-    expect(dest.isPublic()).to.be.true;
+    expect(dest.isPrivate()).to.be.false;
     await expect(dest.update(source))
       .to.be.rejectedWith('Cannot update public key with private key if subkeys mismatch');
   });

test/general/openpgp.js

@@ -1010,15 +1010,15 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
       };
       const armored = await openpgp.generateKey({ ...opt, format: 'armor' });
       expect((await openpgp.readKey({ armoredKey: armored.privateKey })).isPrivate()).to.be.true;
-      expect((await openpgp.readKey({ armoredKey: armored.publicKey })).isPublic()).to.be.true;
+      expect((await openpgp.readKey({ armoredKey: armored.publicKey })).isPrivate()).to.be.false;
 
       const binary = await openpgp.generateKey({ ...opt, format: 'binary' });
       expect((await openpgp.readKey({ binaryKey: binary.privateKey })).isPrivate()).to.be.true;
-      expect((await openpgp.readKey({ binaryKey: binary.publicKey })).isPublic()).to.be.true;
+      expect((await openpgp.readKey({ binaryKey: binary.publicKey })).isPrivate()).to.be.false;
 
       const { privateKey, publicKey } = await openpgp.generateKey({ ...opt, format: 'object' });
       expect(privateKey.isPrivate()).to.be.true;
-      expect(publicKey.isPublic()).to.be.true;
+      expect(publicKey.isPrivate()).to.be.false;
     });
   });
 
@@ -1036,15 +1036,15 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
       };
       const armored = await openpgp.reformatKey({ ...opt, format: 'armor' });
       expect((await openpgp.readKey({ armoredKey: armored.privateKey })).isPrivate()).to.be.true;
-      expect((await openpgp.readKey({ armoredKey: armored.publicKey })).isPublic()).to.be.true;
+      expect((await openpgp.readKey({ armoredKey: armored.publicKey })).isPrivate()).to.be.false;
 
       const binary = await openpgp.reformatKey({ ...opt, format: 'binary' });
       expect((await openpgp.readKey({ binaryKey: binary.privateKey })).isPrivate()).to.be.true;
-      expect((await openpgp.readKey({ binaryKey: binary.publicKey })).isPublic()).to.be.true;
+      expect((await openpgp.readKey({ binaryKey: binary.publicKey })).isPrivate()).to.be.false;
 
       const { privateKey, publicKey } = await openpgp.reformatKey({ ...opt, format: 'object' });
       expect(privateKey.isPrivate()).to.be.true;
-      expect(publicKey.isPublic()).to.be.true;
+      expect(publicKey.isPrivate()).to.be.false;
     });
   });
 
@@ -1058,15 +1058,15 @@ module.exports = () => describe('OpenPGP.js public api tests', function() {
 
       const armored = await openpgp.revokeKey({ key, format: 'armor' });
       expect((await openpgp.readKey({ armoredKey: armored.privateKey })).isPrivate()).to.be.true;
-      expect((await openpgp.readKey({ armoredKey: armored.publicKey })).isPublic()).to.be.true;
+      expect((await openpgp.readKey({ armoredKey: armored.publicKey })).isPrivate()).to.be.false;
 
       const binary = await openpgp.revokeKey({ key, format: 'binary' });
       expect((await openpgp.readKey({ binaryKey: binary.privateKey })).isPrivate()).to.be.true;
-      expect((await openpgp.readKey({ binaryKey: binary.publicKey })).isPublic()).to.be.true;
+      expect((await openpgp.readKey({ binaryKey: binary.publicKey })).isPrivate()).to.be.false;
 
       const { privateKey, publicKey } = await openpgp.revokeKey({ key, format: 'object' });
       expect(privateKey.isPrivate()).to.be.true;
-      expect(publicKey.isPublic()).to.be.true;
+      expect(publicKey.isPrivate()).to.be.false;
     });
   });
 

test/typescript/definitions.ts

@@ -32,11 +32,26 @@ import {
   expect(await readKeys({ armoredKeys: publicKeyArmored })).to.have.lengthOf(1);
   const parsedKey: Key = await readKey({ armoredKey: publicKeyArmored });
   expect(parsedKey.armor(config)).to.equal(publicKeyArmored);
-  expect(parsedKey.isPublic()).to.be.true;
+  expect(parsedKey.isPrivate()).to.be.false;
   const parsedPrivateKey: PrivateKey = await readPrivateKey({ armoredKey: privateKeyArmored });
   expect(parsedPrivateKey.isPrivate()).to.be.true;
   const parsedBinaryPrivateKey: PrivateKey = await readPrivateKey({ binaryKey: privateKeyBinary });
   expect(parsedBinaryPrivateKey.isPrivate()).to.be.true;
+  // a generic Key can be directly used as PublicKey, since both classes have the same properties
+  // eslint-disable-next-line no-unused-vars
+  const unusedPublicKey: PublicKey = parsedKey;
+
+  // Check PrivateKey type inference
+  if (parsedKey.isPrivate()) {
+    expect(parsedKey.isDecrypted()).to.be.true;
+  } else {
+    // @ts-expect-error isDecrypted is not defined for public keys
+    try { parsedKey.isDecrypted(); } catch (e) {}
+  }
+  (await privateKey.update(privateKey)).isDecrypted();
+  (await privateKey.toPublic().update(privateKey)).isDecrypted();
+  // @ts-expect-error isDecrypted is not defined for public keys
+  try { (await privateKey.toPublic().update(privateKey.toPublic())).isDecrypted(); } catch (e) {}
 
   // Revoke keys
   await revokeKey({ key: privateKey });