Merge pull request #505 from bheesham/handle-email-weirdness

denyRegistrationByEmail didn't handle existing users

Author: bheesham

__mocks__/auth0.js

@@ -0,0 +1,2 @@
+const auth0 = jest.createMockFromModule("auth0");
+module.exports = auth0;

tf/actions-pre-user-registration.tf

@@ -1,18 +1,31 @@
 resource "auth0_trigger_actions" "pre_user_registration_flow" {
   trigger = "pre-user-registration"
   actions {
-    id           = auth0_action.deny_registration.id
-    display_name = auth0_action.deny_registration.name
+    id           = auth0_action.deny_registration_by_email.id
+    display_name = auth0_action.deny_registration_by_email.name
   }
 }
 
-resource "auth0_action" "deny_registration" {
-  name    = "denyRegistration"
+# DEBT(bhee): Maybe mint new secrets for this action.
+resource "auth0_action" "deny_registration_by_email" {
+  name    = "denyRegistrationByEmail"
   runtime = "node22"
   deploy  = true
-  code    = file("${path.module}/actions/denyRegistration.js")
+  code    = file("${path.module}/actions/denyRegistrationByEmail.js")
   supported_triggers {
     id      = "pre-user-registration"
     version = "v2"
   }
+  dependencies {
+    name    = "auth0"
+    version = "4.9.0"
+  }
+  secrets {
+    name  = "mgmtClientId"
+    value = local.parsed_secrets["linkUserByEmail_mgmtClientId"]
+  }
+  secrets {
+    name  = "mgmtClientSecret"
+    value = local.parsed_secrets["linkUserByEmail_mgmtClientSecret"]
+  }
 }

tf/actions/denyRegistration.js

@@ -0,0 +1,72 @@
+// Reject users from registering for an application (by client id) using email.
+//
+// This is a workaround for not being able to disable new registrations by
+// email. If we instead disabled the email connection then we'd break logins
+// for users who already have an account.
+//
+// DEBT(bhee): LDAP's connection name is
+// * `Mozilla-LDAP` on prod;
+// * `Mozilla-LDAP-Dev` on dev.
+//
+// If we need to deny registrations on those, for some reason, we'll need to
+// think of a better way. Connection Ids are not stable across tenants either.
+
+const auth0 = require("auth0");
+
+exports.onExecutePreUserRegistration = async (event, api) => {
+  const CLIENT_CONNECTIONS_DENYLIST = [
+    // Matrix, IAM-1617
+    "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F",
+  ];
+
+  if (event.connection.name != "email") {
+    return;
+  }
+
+  // Exit early if it's not an application we care about.
+  if (!CLIENT_CONNECTIONS_DENYLIST.includes(event.client.client_id)) {
+    return;
+  }
+
+  var domain;
+  if (event.tenant.id == "dev") {
+    domain = "dev.mozilla-dev.auth0.com";
+  } else if (event.tenant.id == "auth") {
+    domain = "auth.mozilla.auth0.com";
+  } else {
+    return api.access.deny(
+      `Action denyRegistrationByEmail does not support this tenant.`
+    );
+  }
+
+  const mgmt = new auth0.ManagementClient({
+    domain,
+    clientId: event.secrets.mgmtClientId,
+    clientSecret: event.secrets.mgmtClientSecret,
+    scope: "read:users",
+  });
+
+  // On an API error we default to doors closed.
+  const userExists = async (mgmt, email) => {
+    try {
+      const response = await mgmt.usersByEmail.getByEmail({
+        email,
+      });
+      return response.data.length > 0;
+    } catch (err) {
+      return false;
+    }
+  };
+
+  // The user exists already! Let them in.
+  if (
+    (await userExists(mgmt, event.user.email)) ||
+    (await userExists(mgmt, event.user.email.toLowerCase()))
+  ) {
+    return;
+  }
+
+  return api.access.deny(
+    `Not allowed to register for ${event.client.name} using ${event.connection.name}.`
+  );
+};

tf/actions/denyRegistrationByEmail.js

@@ -0,0 +1,69 @@
+const auth0 = require("auth0");
+const _ = require("lodash");
+const eventObj = require("./modules/event.json");
+const { emailUser, ldapUser } = require("./modules/users.js");
+const {
+  onExecutePreUserRegistration,
+} = require("../actions/denyRegistrationByEmail.js");
+
+beforeEach(() => {
+  _event = _.cloneDeep(eventObj);
+  _event.secrets = {
+    mgmtClientId: "fake",
+    mgmtClientSecrets: "fake",
+  };
+  _emailUser = _.cloneDeep(emailUser);
+
+  api = {
+    access: {
+      deny: jest.fn(),
+    },
+  };
+
+  mockManagementClient = {
+    usersByEmail: {
+      getByEmail: jest.fn(),
+    },
+  };
+
+  auth0.ManagementClient = jest.fn(() => mockManagementClient);
+});
+
+afterEach(() => {
+  jest.clearAllMocks();
+});
+
+test("Should not deny registration an app we haven't specified", async () => {
+  await onExecutePreUserRegistration(_event, api);
+  expect(api.access.deny).not.toHaveBeenCalled();
+});
+
+test("Should deny a new registration for Matrix", async () => {
+  _event.connection.name = "email";
+  _event.client.client_id = "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F";
+  mockManagementClient.usersByEmail.getByEmail.mockReturnValue(
+    Promise.resolve({ data: [] })
+  );
+  await onExecutePreUserRegistration(_event, api);
+  expect(api.access.deny).toHaveBeenCalled();
+});
+
+test("Should allow an email login for Matrix", async () => {
+  _event.connection.name = "email";
+  _event.client.client_id = "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F";
+  mockManagementClient.usersByEmail.getByEmail
+    .mockReturnValueOnce(Promise.resolve({ data: [_emailUser] }))
+    .mockReturnValueOnce(Promise.resolve({ data: [] }));
+  await onExecutePreUserRegistration(_event, api);
+  expect(api.access.deny).not.toHaveBeenCalled();
+});
+
+test("Should deny an email login for Matrix in the face of an exception", async () => {
+  _event.connection.name = "email";
+  _event.client.client_id = "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F";
+  mockManagementClient.usersByEmail.getByEmail.mockImplementation(async () => {
+    throw new Error("test");
+  });
+  await onExecutePreUserRegistration(_event, api);
+  expect(api.access.deny).toHaveBeenCalled();
+});