Skip to content

Commit 2edf3bd

Browse files
committed
denyRegistrationByEmail didn't handle existing users
When logging in by email we don't consider if the user exists already. I mistakenly assumed Auth0 would handle this for us, but as it turns out: nope. Jira: IAM-1617
1 parent bc87db6 commit 2edf3bd

6 files changed

Lines changed: 161 additions & 63 deletions

File tree

__mocks__/auth0.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
const auth0 = jest.createMockFromModule("auth0");
2+
module.exports = auth0;
Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,31 @@
11
resource "auth0_trigger_actions" "pre_user_registration_flow" {
22
trigger = "pre-user-registration"
33
actions {
4-
id = auth0_action.deny_registration.id
5-
display_name = auth0_action.deny_registration.name
4+
id = auth0_action.deny_registration_by_email.id
5+
display_name = auth0_action.deny_registration_by_email.name
66
}
77
}
88

9-
resource "auth0_action" "deny_registration" {
10-
name = "denyRegistration"
9+
# DEBT(bhee): Maybe mint new secrets for this action.
10+
resource "auth0_action" "deny_registration_by_email" {
11+
name = "denyRegistrationByEmail"
1112
runtime = "node22"
1213
deploy = true
13-
code = file("${path.module}/actions/denyRegistration.js")
14+
code = file("${path.module}/actions/denyRegistrationByEmail.js")
1415
supported_triggers {
1516
id = "pre-user-registration"
1617
version = "v2"
1718
}
19+
dependencies {
20+
name = "auth0"
21+
version = "4.9.0"
22+
}
23+
secrets {
24+
name = "mgmtClientId"
25+
value = local.parsed_secrets["linkUserByEmail_mgmtClientId"]
26+
}
27+
secrets {
28+
name = "mgmtClientSecret"
29+
value = local.parsed_secrets["linkUserByEmail_mgmtClientSecret"]
30+
}
1831
}

tf/actions/denyRegistration.js

Lines changed: 0 additions & 32 deletions
This file was deleted.
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Reject users from registering for an application (by client id) using email.
2+
//
3+
// This is a workaround for not being able to disable new registrations by
4+
// email. If we instead disabled the email connection then we'd break logins
5+
// for users who already have an account.
6+
//
7+
// DEBT(bhee): LDAP's connection name is
8+
// * `Mozilla-LDAP` on prod;
9+
// * `Mozilla-LDAP-Dev` on dev.
10+
//
11+
// If we need to deny registrations on those, for some reason, we'll need to
12+
// think of a better way. Connection Ids are not stable across tenants either.
13+
14+
const auth0 = require("auth0");
15+
16+
exports.onExecutePreUserRegistration = async (event, api) => {
17+
const CLIENT_CONNECTIONS_DENYLIST = [
18+
// Matrix, IAM-1617
19+
"pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F",
20+
];
21+
22+
if (event.connection.name != "email") {
23+
return;
24+
}
25+
26+
// Exit early if it's not an application we care about.
27+
if (!CLIENT_CONNECTIONS_DENYLIST.includes(event.client.client_id)) {
28+
return;
29+
}
30+
31+
var domain;
32+
if (event.tenant.id == "dev") {
33+
domain = "dev.mozilla-dev.auth0.com";
34+
} else if (event.tenant.id == "auth") {
35+
domain = "auth.mozilla.auth0.com";
36+
} else {
37+
return api.access.deny(
38+
`Action denyRegistrationByEmail does not support this tenant.`
39+
);
40+
}
41+
42+
const mgmt = new auth0.ManagementClient({
43+
domain,
44+
clientId: event.secrets.mgmtClientId,
45+
clientSecret: event.secrets.mgmtClientSecret,
46+
scope: "read:users",
47+
});
48+
49+
// On an API error we default to doors closed.
50+
const userExists = async (mgmt, email) => {
51+
try {
52+
const response = await mgmt.usersByEmail.getByEmail({
53+
email,
54+
});
55+
return response.data.length > 0;
56+
} catch (err) {
57+
return false;
58+
}
59+
};
60+
61+
// The user exists already! Let them in.
62+
if (
63+
(await userExists(mgmt, event.user.email)) ||
64+
(await userExists(mgmt, event.user.email.toLowerCase()))
65+
) {
66+
return;
67+
}
68+
69+
return api.access.deny(
70+
`Not allowed to register for ${event.client.name} using ${event.connection.name}.`
71+
);
72+
};

tf/tests/denyRegistration.test.js

Lines changed: 0 additions & 26 deletions
This file was deleted.
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
const auth0 = require("auth0");
2+
const _ = require("lodash");
3+
const eventObj = require("./modules/event.json");
4+
const { emailUser, ldapUser } = require("./modules/users.js");
5+
const {
6+
onExecutePreUserRegistration,
7+
} = require("../actions/denyRegistrationByEmail.js");
8+
9+
beforeEach(() => {
10+
_event = _.cloneDeep(eventObj);
11+
_event.secrets = {
12+
mgmtClientId: "fake",
13+
mgmtClientSecrets: "fake",
14+
};
15+
_emailUser = _.cloneDeep(emailUser);
16+
17+
api = {
18+
access: {
19+
deny: jest.fn(),
20+
},
21+
};
22+
23+
mockManagementClient = {
24+
usersByEmail: {
25+
getByEmail: jest.fn(),
26+
},
27+
};
28+
29+
auth0.ManagementClient = jest.fn(() => mockManagementClient);
30+
});
31+
32+
afterEach(() => {
33+
jest.clearAllMocks();
34+
});
35+
36+
test("Should not deny registration an app we haven't specified", async () => {
37+
await onExecutePreUserRegistration(_event, api);
38+
expect(api.access.deny).not.toHaveBeenCalled();
39+
});
40+
41+
test("Should deny a new registration for Matrix", async () => {
42+
_event.connection.name = "email";
43+
_event.client.client_id = "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F";
44+
mockManagementClient.usersByEmail.getByEmail.mockReturnValue(
45+
Promise.resolve({ data: [] })
46+
);
47+
await onExecutePreUserRegistration(_event, api);
48+
expect(api.access.deny).toHaveBeenCalled();
49+
});
50+
51+
test("Should allow an email login for Matrix", async () => {
52+
_event.connection.name = "email";
53+
_event.client.client_id = "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F";
54+
mockManagementClient.usersByEmail.getByEmail
55+
.mockReturnValueOnce(Promise.resolve({ data: [_emailUser] }))
56+
.mockReturnValueOnce(Promise.resolve({ data: [] }));
57+
await onExecutePreUserRegistration(_event, api);
58+
expect(api.access.deny).not.toHaveBeenCalled();
59+
});
60+
61+
test("Should deny an email login for Matrix in the face of an exception", async () => {
62+
_event.connection.name = "email";
63+
_event.client.client_id = "pFf6sBIfp4n3Wcs3F9Q7a9ry8MTrbi2F";
64+
mockManagementClient.usersByEmail.getByEmail.mockImplementation(async () => {
65+
throw new Error("test");
66+
});
67+
await onExecutePreUserRegistration(_event, api);
68+
expect(api.access.deny).toHaveBeenCalled();
69+
});

0 commit comments

Comments
 (0)