Attend to review comments

Author: brizental

glean/src/plugins/encryption.ts

@@ -12,20 +12,40 @@ import { JSONObject } from "../core/utils";
 import CoreEvents from "../core/events";
 
 /**
- * A plugin that encrypts the payload of pings before they are stored and sent.
+ * A plugin that listens for the `afterPingCollection` event and encrypts **all** outgoing pings
+ * with the JWK provided upon initialization.
+ *
+ * This plugin will modify the schema of outgoing pings to:
+ *
+ * ```json
+ * {
+ *  payload: "<encrypted-payload>"
+ * }
+ * ```
  */
 class PingEncryptionPlugin extends Plugin<typeof CoreEvents["afterPingCollection"]> {
-  constructor(readonly jwk: JWK) {
+  /**
+   * Creates a new PingEncryptionPlugin instance.
+   *
+   * @param jwk The JWK that will be used to encode outgoing ping payloads.
+   * @param alg The algorithm this plugin will use for parsing the JWK. If this argument is not present,
+   *            we will look for the `alg` key in the JWK. If neither is present we defaut to "ECDH-ES".
+   */
+  constructor(private jwk: JWK, private alg?: string) {
     super(CoreEvents["afterPingCollection"].name, "pingEncryptionPlugin");
+
+    if (!alg) {
+      this.alg = jwk.alg ? jwk.alg : "ECDH-ES";
+    }
   }
 
   async action(payload: PingPayload): Promise<JSONObject> {
-    const key = await parseJwk(this.jwk);
+    const key = await parseJwk(this.jwk, this.alg);
     const encoder = new TextEncoder();
     const encodedPayload = await new CompactEncrypt(encoder.encode(JSON.stringify(payload)))
       .setProtectedHeader({
         kid: this.jwk.kid,
-        alg: this.jwk.alg,
+        alg: this.alg,
         enc: "A256GCM",
         typ: "JWE",
       })

glean/tests/plugins/encryption.spec.ts

@@ -13,10 +13,12 @@ import PingEncryptionPlugin from "../../src/plugins/encryption";
 const sandbox = sinon.createSandbox();
 
 describe("PingEncryptionPlugin", function() {
+  // eslint-disable-next-line mocha/no-hooks-for-single-case
   beforeEach(async function() {
     await Glean.testResetGlean("something something");
   });
 
+  // eslint-disable-next-line mocha/no-hooks-for-single-case
   afterEach(function () {
     sandbox.restore();
   });
@@ -26,25 +28,31 @@ describe("PingEncryptionPlugin", function() {
     sandbox.stub(Glean["pingUploader"], "triggerUpload").callsFake(() => Promise.resolve());
 
     await Glean.testUninitialize();
+    await Glean.testInitialize(
+      "something something",
+      true,
+      {
+        plugins: [
+          new PingEncryptionPlugin({
+            "crv": "P-256",
+            "kid": "test",
+            "kty": "EC",
+            "x": "Q20tsJdrryWJeuPXTM27wIPb_YbsdYPpkK2N9O6aXwM",
+            "y": "1onW1swaCcN1jkmkIwhXpCm55aMP8GRJln5E8WQKLJk"
+          })
+        ]
+      }
+    );
 
-    const plugin = new PingEncryptionPlugin({
-      "kid": "test",
-      "alg": "ECDH-ES",
-      "crv": "P-256",
-      "kty": "EC",
-      "x": "Qqihp7EryDN2-qQ-zuDPDpy5mJD5soFBDZmzPWTmjwk",
-      "y": "PiEQVUlywi2bEsA3_5D0VFrCHClCyUlLW52ajYs-5uc"
-    });
-    await Glean.testInitialize("something something", true, { plugins: [ plugin ]});
     const ping = new PingType({
       name: "ping",
       includeClientId: true,
       sendIfEmpty: true,
     });
-
     await PingMaker.collectAndStorePing("ident", ping);
     const recordedPing = (await Glean.pingsDatabase.getAllPings())["ident"];
-    
+
     assert.ok("payload" in recordedPing.payload);
+    assert.strictEqual(Object.keys(recordedPing.payload).length, 1);
   });
 });