chore: Refactor @Encrypted decorator (#7381)

* chore: Simplify encrypted decorator

* fix: Correctly handle and type nullable encrypted fields

* docs

app/models/WebhookSubscription.ts

@@ -19,7 +19,7 @@ class WebhookSubscription extends Model {
 
   @Field
   @observable
-  secret: string;
+  secret: string | null;
 
   @Field
   @observable

plugins/webhooks/client/components/WebhookSubscriptionForm.tsx

@@ -154,7 +154,7 @@ type Props = {
 interface FormData {
   name: string;
   url: string;
-  secret: string;
+  secret: string | null;
   events: string[];
 }
 
@@ -177,7 +177,9 @@ function WebhookSubscriptionForm({ handleSubmit, webhookSubscription }: Props) {
       events: webhookSubscription ? [...webhookSubscription.events] : [],
       name: webhookSubscription?.name,
       url: webhookSubscription?.url,
-      secret: webhookSubscription?.secret ?? generateSigningSecret(),
+      secret: webhookSubscription
+        ? webhookSubscription?.secret
+        : generateSigningSecret(),
     },
   });
 

server/models/IntegrationAuthentication.ts

@@ -10,10 +10,7 @@ import { IntegrationService } from "@shared/types";
 import Team from "./Team";
 import User from "./User";
 import IdModel from "./base/IdModel";
-import Encrypted, {
-  getEncryptedColumn,
-  setEncryptedColumn,
-} from "./decorators/Encrypted";
+import Encrypted from "./decorators/Encrypted";
 import Fix from "./decorators/Fix";
 
 @Table({ tableName: "authentications", modelName: "authentication" })
@@ -30,23 +27,11 @@ class IntegrationAuthentication extends IdModel<
 
   @Column(DataType.BLOB)
   @Encrypted
-  get token() {
-    return getEncryptedColumn(this, "token");
-  }
-
-  set token(value: string) {
-    setEncryptedColumn(this, "token", value);
-  }
+  token: string;
 
   @Column(DataType.BLOB)
   @Encrypted
-  get refreshToken() {
-    return getEncryptedColumn(this, "refreshToken");
-  }
-
-  set refreshToken(value: string) {
-    setEncryptedColumn(this, "refreshToken", value);
-  }
+  refreshToken: string;
 
   // associations
 

server/models/User.ts

@@ -56,10 +56,7 @@ import Team from "./Team";
 import UserAuthentication from "./UserAuthentication";
 import UserMembership from "./UserMembership";
 import ParanoidModel from "./base/ParanoidModel";
-import Encrypted, {
-  setEncryptedColumn,
-  getEncryptedColumn,
-} from "./decorators/Encrypted";
+import Encrypted from "./decorators/Encrypted";
 import Fix from "./decorators/Fix";
 import IsUrlOrRelativePath from "./validators/IsUrlOrRelativePath";
 import Length from "./validators/Length";
@@ -143,13 +140,7 @@ class User extends ParanoidModel<
 
   @Column(DataType.BLOB)
   @Encrypted
-  get jwtSecret() {
-    return getEncryptedColumn(this, "jwtSecret");
-  }
-
-  set jwtSecret(value: string) {
-    setEncryptedColumn(this, "jwtSecret", value);
-  }
+  jwtSecret: string;
 
   @IsDate
   @Column

server/models/UserAuthentication.ts

@@ -18,10 +18,7 @@ import Logger from "@server/logging/Logger";
 import AuthenticationProvider from "./AuthenticationProvider";
 import User from "./User";
 import IdModel from "./base/IdModel";
-import Encrypted, {
-  getEncryptedColumn,
-  setEncryptedColumn,
-} from "./decorators/Encrypted";
+import Encrypted from "./decorators/Encrypted";
 import Fix from "./decorators/Fix";
 
 @Table({ tableName: "user_authentications", modelName: "user_authentication" })
@@ -35,23 +32,11 @@ class UserAuthentication extends IdModel<
 
   @Column(DataType.BLOB)
   @Encrypted
-  get accessToken() {
-    return getEncryptedColumn(this, "accessToken");
-  }
-
-  set accessToken(value: string) {
-    setEncryptedColumn(this, "accessToken", value);
-  }
+  accessToken: string;
 
   @Column(DataType.BLOB)
   @Encrypted
-  get refreshToken() {
-    return getEncryptedColumn(this, "refreshToken");
-  }
-
-  set refreshToken(value: string) {
-    setEncryptedColumn(this, "refreshToken", value);
-  }
+  refreshToken: string;
 
   @Column
   providerId: string;

server/models/WebhookSubscription.ts

@@ -1,5 +1,5 @@
 import crypto from "crypto";
-import isEmpty from "lodash/isEmpty";
+import isNil from "lodash/isNil";
 import {
   InferAttributes,
   InferCreationAttributes,
@@ -23,10 +23,7 @@ import { Event } from "@server/types";
 import Team from "./Team";
 import User from "./User";
 import ParanoidModel from "./base/ParanoidModel";
-import Encrypted, {
-  setEncryptedColumn,
-  getEncryptedColumn,
-} from "./decorators/Encrypted";
+import Encrypted from "./decorators/Encrypted";
 import Fix from "./decorators/Fix";
 import Length from "./validators/Length";
 
@@ -65,19 +62,9 @@ class WebhookSubscription extends ParanoidModel<
   events: string[];
 
   @AllowNull
-  @Encrypted
   @Column(DataType.BLOB)
-  get secret() {
-    const val = getEncryptedColumn(this, "secret");
-    // Turns out that `val` evals to `{}` instead
-    // of `null` even if secret's value in db is `null`.
-    // https://github.com/defunctzombie/sequelize-encrypted/blob/c3854e76ae4b80318c8f10f94e6c898c67659ca6/index.js#L30-L33 explains it possibly.
-    return isEmpty(val) ? "" : val;
-  }
-
-  set secret(value: string) {
-    setEncryptedColumn(this, "secret", value);
-  }
+  @Encrypted
+  secret: string | null;
 
   // associations
 
@@ -152,7 +139,7 @@ class WebhookSubscription extends ParanoidModel<
    * @returns the signature as a string
    */
   public signature = (payload: string) => {
-    if (isEmpty(this.secret)) {
+    if (isNil(this.secret)) {
       return;
     }
 

server/models/decorators/Encrypted.ts

@@ -1,4 +1,6 @@
+import isEmpty from "lodash/isEmpty";
 import isNil from "lodash/isNil";
+import { getAttributes } from "sequelize-typescript";
 import Logger from "@server/logging/Logger";
 import vaults from "@server/storage/vaults";
 
@@ -10,45 +12,55 @@ const key = "sequelize:vault";
  * @Column(DataType.BLOB) annotation.
  */
 export default function Encrypted(target: any, propertyKey: string) {
+  // Ensure that the Encrypted decorator is the first decorator applied to the property, we can check
+  // this by looking at the attributes of the target and checking if the propertyKey is already defined.
+  if (getAttributes(target)[propertyKey]) {
+    Logger.fatal(
+      `The Encrypted decorator must be the first decorator applied to the property ${propertyKey} in ${target.constructor.name}`,
+      new Error()
+    );
+  }
+
   Reflect.defineMetadata(key, vaults().vault(propertyKey), target, propertyKey);
-}
 
-/**
- * Get the value of an encrypted column given the target and the property key.
- */
-export function getEncryptedColumn(target: any, propertyKey: string): string {
-  if (!target.getDataValue(propertyKey)) {
-    return "";
-  }
-  try {
-    return Reflect.getMetadata(key, target, propertyKey).get.call(target);
-  } catch (err) {
-    if (err.message.includes("Unexpected end of JSON input")) {
-      return "";
-    }
-    if (err.message.includes("bad decrypt")) {
-      Logger.error(
-        `Failed to decrypt database column (${propertyKey}). The SECRET_KEY environment variable may have changed since installation.`,
-        err
-      );
-      process.exit(1);
-    }
+  return {
+    get() {
+      const attributeOptions = getAttributes(target);
+      const defaultValue = attributeOptions[propertyKey].allowNull ? null : "";
 
-    throw err;
-  }
-}
+      if (!this.getDataValue(propertyKey)) {
+        return defaultValue;
+      }
+      try {
+        const value = Reflect.getMetadata(key, this, propertyKey).get.call(
+          this
+        );
 
-/**
- * Set the value of an encrypted column given the target and the property key.
- */
-export function setEncryptedColumn(
-  target: any,
-  propertyKey: string,
-  value: string
-) {
-  if (isNil(value)) {
-    target.setDataValue(propertyKey, value);
-  } else {
-    Reflect.getMetadata(key, target, propertyKey).set.call(target, value);
-  }
+        // `value` equals to `{}` instead of `null` if column value in db is `null`. Possibly explained by:
+        // https://github.com/defunctzombie/sequelize-encrypted/blob/c3854e76ae4b80318c8f10f94e6c898c67659ca6/index.js#L30-L33
+        return isEmpty(value) && typeof value === "object"
+          ? defaultValue
+          : value;
+      } catch (err) {
+        if (err.message.includes("Unexpected end of JSON input")) {
+          return defaultValue;
+        }
+        if (err.message.includes("bad decrypt")) {
+          Logger.fatal(
+            `Failed to decrypt database column (${propertyKey}). The SECRET_KEY environment variable may have changed since installation.`,
+            err
+          );
+        }
+
+        throw err;
+      }
+    },
+    set(value: string | null) {
+      if (isNil(value)) {
+        this.setDataValue(propertyKey, value);
+      } else {
+        Reflect.getMetadata(key, this, propertyKey).set.call(this, value);
+      }
+    },
+  } as any;
 }