2676 prevent higher access users from changing email to personal by arshiamasih · Pull Request #3302 · hackforla/tdm-calculator

arshiamasih · 2026-06-13T00:25:48Z

Fixes Dev: Limit Users with Higher Level Access from Changing Emails to Personal Email Accounts #2676

What changes did you make?

The original PR issue was to limit admin users from updating their email to personal email address. However there were a few other issues in account updates that were coupled closely enough to be handled in this PR.

adds a domain whitelist to restrict admin/security admin users from updating email to non approved domains
fixes errors to handle case for existing emails failure
adds explicit error handling for email verification confirmation errors
adds integration testing for updateAccount to check for standard user versus admin user personal email updates

Why did you make the changes (we will use this info to test)?

To reduce the chance that an LA City employee will retain access to admin or security permissions after leaving the city.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

Admin attempt to change to personal email - SUCCEEDS

change in PR: blocks update requests to non-approved email domains

Security attempt to change to personal email - SUCCEEDS

change in PR: blocks update requests to non-approved email domains

develop-security-peronal-success.mov

Attempt to change to another existing email - BLOCKED with a catch all error message

change in PR: the error was caught in a generic catch all error message instead of the explicit "existing email" as the back end response code did not match the front end error handling

SMTP / TDM Google mailer failure - vague error message:

Visuals after changes are applied

Security Admin change email actions:

Security Admin attempt to change to personal email - BLOCKED with explicit error message

Database NO update on personal email change attempt (DBeaver):
https://github.com/user-attachments/assets/bee5c88c-2288-4dcc-b169-2a10978f03b4

Security Admin attempt to change to existing email - BLOCKED with correct explicit error message

Database NO update on existing email change attempt (DBeaver):
https://github.com/user-attachments/assets/055bc5fa-5a2d-4dac-8c3c-40a3c979d52c

Security success on allowed email domain - SUCCESS confirmation:

Database success update on approved email domain (DBeaver):

Successful login with changed email (confirming no breaking changes to login flow):

Screen.Recording.2026-07-09.at.11.36.37.AM.mov

Admin change email actions:

Admin attempt to change to personal email - BLOCKED with explicit error message

Admin attempt to change to existing email - BLOCKED with correct explicit error message

Admin success on allowed email domain - SUCCESS confirmation:

Database update (DBeaver):

Non-admin change personal email action - NOT BLOCKED:

SMTP / TDM Google mailer failure - added explicit error message

…nging-email-to-personal

…date account component

…que constraint violations

…ndition

…into develop

…email constraints

… error message

…nent to handle email confirmation failures expliicitly

arshiamasih · 2026-07-05T17:30:58Z

This is a placeholder config file until we move domain lists to .env file.

Another option could be to move these lists to the database - persisting in DB seems a bit over engineered for the currents needs imo, but I am open to considering if there are preferences towards it.

I'm going to put the list of domains in the .env file, so they can be altered as necessary by the devops people - who will be able to change runtime environment variable settings without having to edit the database or code. I'll do this as a small change to your PR, and update the .env contents settings in the 1Passaword vault(s).

arshiamasih · 2026-07-05T17:32:07Z

+    const user = await selectById(model.id);
+    if (!user) {
+      const error = new Error("User record not found.");
+      error.code = "USER_NOT_FOUND";
+      throw error;


I believe the user sessions is accessible from the middleware layer so perhaps this is redundant but also seems more secure.

I'd have to check to make sure the user object from the middleware has the same structure as the user object from 'selectById', but it probably does., so it should not make a difference where you get the user info. The middleware gets it from the seure JWT header in the HTTP request so it better be secure, or we have bigger problems. I'm ok with what you did here.

arshiamasih · 2026-07-05T17:34:54Z

+
+const validateUniqueEmail = async (email, currentUserId) => {
+  const trimmedEmail = email.toLowerCase().trim();
+  const existingEmailCheck = await selectByEmail(trimmedEmail);
+
+  if (existingEmailCheck && existingEmailCheck.id !== currentUserId) {
+    const error = new Error(
+      `The email ${email} is already in use by another account.`
+    );
+    error.code = "ERR_DUPLICATE_EMAIL";
+    throw error;
+  }
+};


The Login_Update stored procedure also checks for email uniqueness by looking at the login table, but imo we should be validating reqs upfront before we attempt a DB update- the stored procedure then acts as a secondary safeguard check. LMK what you think.

That's good.

…nging-email-to-personal

…into develop

entrotech

I'm going to make a few small changes to the PR before merging it, so the domain list comes from the .env file as you suggested, but it seems to be working well and gives good error messages when they try to enter a duplicate email or use a restricted domain.

We will need to create another issue to prevent someone who has security admin rights from granting roles to a user from the security admin page if their user name is not associated with an allowed domain.

entrotech · 2026-07-16T00:25:29Z

+
+const validateUniqueEmail = async (email, currentUserId) => {
+  const trimmedEmail = email.toLowerCase().trim();
+  const existingEmailCheck = await selectByEmail(trimmedEmail);
+
+  if (existingEmailCheck && existingEmailCheck.id !== currentUserId) {
+    const error = new Error(
+      `The email ${email} is already in use by another account.`
+    );
+    error.code = "ERR_DUPLICATE_EMAIL";
+    throw error;
+  }
+};


That's good.

entrotech · 2026-07-16T00:27:15Z

+
+const validateUniqueEmail = async (email, currentUserId) => {
+  const trimmedEmail = email.toLowerCase().trim();
+  const existingEmailCheck = await selectByEmail(trimmedEmail);
+
+  if (existingEmailCheck && existingEmailCheck.id !== currentUserId) {
+    const error = new Error(
+      `The email ${email} is already in use by another account.`
+    );
+    error.code = "ERR_DUPLICATE_EMAIL";
+    throw error;
+  }
+};


That's good.

entrotech · 2026-07-16T00:31:12Z

+    const user = await selectById(model.id);
+    if (!user) {
+      const error = new Error("User record not found.");
+      error.code = "USER_NOT_FOUND";
+      throw error;


I'd have to check to make sure the user object from the middleware has the same structure as the user object from 'selectById', but it probably does., so it should not make a difference where you get the user info. The middleware gets it from the seure JWT header in the HTTP request so it better be secure, or we have bigger problems. I'm ok with what you did here.

entrotech · 2026-07-16T00:34:32Z

I'm going to put the list of domains in the .env file, so they can be altered as necessary by the devops people - who will be able to change runtime environment variable settings without having to edit the database or code. I'll do this as a small change to your PR, and update the .env contents settings in the 1Passaword vault(s).

…nging-email-to-personal

arshiamasih and others added 21 commits June 10, 2026 13:18

Fix update account service response messages

6a8e38a

UAT env shouls show uAT badge in app header (#3298) (#3299)

c1483c4

merge develop into uat branch

1d7bdbc

Add basic nodejs helmet configuration

0e67bb6

Attempt to implment CSP security header

1d9b7c3

Relax CSP configuration

e547519

Re-disable CSP config in helmet

f706f54

First attempt at fixing Subresource Integrity issue

f01c7d7

Merge branch 'develop' into 2676-prevent-higher-access-users-from-cha…

5177cb4

…nging-email-to-personal

Add email validation logic to update account service and handle in up…

7740726

…date account component

Try relaxed helmet config

6949cce

Try relaxed helmet config

ceba644

Try relaxed helmet config

2644f24

Try relaxed helmet config

3b7367d

Try relaxed helmet config

7755be8

Try relaxed helmet config

52db0ea

Try relaxed helmet config

934b0a3

Try relaxed helmet config

7a4cf8d

Try relaxed helmet config

728376b

Try relaxed helmet config

e06a52f

Fix update account component signature and add error handling for uni…

82f6fdc

…que constraint violations

arshiamasih marked this pull request as draft June 13, 2026 00:26

entrotech and others added 4 commits June 12, 2026 20:18

Try relaxed helmet config

39d219f

Try relaxed helmet config a

769db1e

Add additional email dupe check as addl safeguard and prevent race co…

e7a48d4

…ndition

Update variable name

4081903

arshiamasih marked this pull request as ready for review June 14, 2026 20:28

arshiamasih requested a review from entrotech June 14, 2026 21:42

arshiamasih added role: back-end Node/Express Development Task level: medium labels Jun 14, 2026

entrotech added 5 commits June 15, 2026 20:37

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

8e392d1

…into develop

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

68d26bb

…into develop

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

53a1c57

…into develop

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

b80c555

…into develop

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

07c4e25

…into develop

arshiamasih marked this pull request as draft June 21, 2026 20:17

arshiamasih added 6 commits June 23, 2026 13:53

Separate logic checks in update account into separate functions

6c004dc

Add update account tests coverage for domain restrictions and unique …

3789dd9

…email constraints

Add comment to move email domain white lists

1002661

Fix nested test

94f32d5

Move approved email domains to config file and update duplicate email…

8d40a7e

… error message

Put email verification in separate function and update frontend compo…

9964c19

…nent to handle email confirmation failures expliicitly

arshiamasih marked this pull request as ready for review June 30, 2026 19:02

arshiamasih commented Jul 5, 2026

View reviewed changes

arshiamasih and others added 8 commits July 5, 2026 10:43

Update dev domain list comment

7c8efa3

Merge branch 'develop' into 2676-prevent-higher-access-users-from-cha…

9d21b52

…nging-email-to-personal

Clean up sending verification email error message

5579c35

Clean up sending verification email error message

5d8fc91

Add TODO comment in allowed domain config file

621acd4

Update TODO comment in allowed domain config file

3eb3467

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

9b71ede

…into develop

Merge branch 'develop' of https://github.com/hackforla/tdm-calculator …

50c37d0

…into develop

entrotech approved these changes Jul 16, 2026

View reviewed changes

entrotech added 3 commits July 15, 2026 17:43

Get ALLOWED_ADMIN_EMAIL_DOMAINS from .env

bced64b

Add env variable for unit test

c47dfd7

Merge branch 'develop' into 2676-prevent-higher-access-users-from-cha…

4a6761b

…nging-email-to-personal

entrotech merged commit fd01290 into develop Jul 21, 2026
7 checks passed

entrotech deleted the 2676-prevent-higher-access-users-from-changing-email-to-personal branch July 21, 2026 23:19

Uh oh!

Uh oh!

Conversation

arshiamasih commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes did you make?

Why did you make the changes (we will use this info to test)?

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Admin attempt to change to personal email - SUCCEEDS

Security attempt to change to personal email - SUCCEEDS

Attempt to change to another existing email - BLOCKED with a catch all error message

SMTP / TDM Google mailer failure - vague error message:

Security Admin change email actions:

Admin change email actions:

Non-admin change personal email action - NOT BLOCKED:

SMTP / TDM Google mailer failure - added explicit error message

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

entrotech left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arshiamasih commented Jun 13, 2026 •

edited

Loading