Skip to content

Fix legacy user accounts with email-as-UUID blocking project membership (#1545)#1546

Open
JasonWildMe wants to merge 2 commits into
mainfrom
fix/user-uuid-email-1545
Open

Fix legacy user accounts with email-as-UUID blocking project membership (#1545)#1546
JasonWildMe wants to merge 2 commits into
mainfrom
fix/user-uuid-email-1545

Conversation

@JasonWildMe
Copy link
Copy Markdown
Collaborator

@JasonWildMe JasonWildMe commented Apr 16, 2026

Fixes #1545.

Root cause

UserCreate.java:103 was calling new User(email), which resolves to the User(String uuid) constructor and stores the email address in the uuid field (which is the JDO primary key per package.jdo:787). A small number of accounts created via this path have user.getUUID() returning an email.

Downstream:

  1. UserGetSimpleJSON serves u.getUUID() as the autocomplete id.
  2. editProject.jsp / createProject.jsp push that email into userIdsToAdd.
  3. ProjectUpdate.addOrRemoveUsersFromProject / ProjectCreate do if (Util.isUUID(userId)) getUserByUUID else getUser(username). The email isn't a UUID, so it falls to the username lookup, which misses if username ≠ email.
  4. Loop silently no-ops but res.success = true is still set. UI shows "Success updating project data." The user is never added.

Changes

  • UserCreate.java: switch to new User(email, Util.generateUUID()) so no new accounts are created with email-as-UUID.
  • User.java: @Deprecated on the single-arg User(String uuid) constructor with a Javadoc note pointing at this issue. Not removed — there are three legitimate callers (SpotterConserveIO, BulkImporter, StandardImport) that pass real UUIDs. Those will be migrated in follow-up tickets.
  • ProjectUpdate.java: factor the user lookup into a shared resolveUser() that adds an getUserByEmailAddress fallback for legacy accounts. Every fallback hit is logged. Surface unresolvedUserIds: [...] in the response JSON so silent misses are visible to the frontend.
  • ProjectCreate.java: use the same resolveUser() helper; same unresolvedUserIds surface. The bug was live in this path too.

What this does NOT do

This is a code patch, not a data migration. Existing rows with email-as-UUID remain in the DB. The resolveUser() fallback keeps those users functional for project add/remove until a proper migration runs.

The migration is deliberately deferred to a separate ticket. It needs:

  • Per-install execution with admin auth, dry-run preview, DB backup prompt
  • Clone-and-swap (can't mutate a JDO primary key in place, and USERS.username is unique so it needs careful sequencing)
  • Rewrite references across Project.users, Project.ownerId (plain string), Organization.members, Encounter.submitters/photographers/informOthers, Occurrence.submitters/informOthers, ImportTask.creator, Collaboration, audit trails
  • OpenSearch reindex of affected encounter and occurrence docs (submitterUserId and viewUsers are stored as the raw id) — not just OpenSearch.setPermissionsNeeded

Diagnostic SQL

SELECT COUNT(*) AS affected_count
FROM "USERS"
WHERE "UUID" !~ '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$';

Test plan

  • Login as project owner on ACW (where the bug was reported) and add a previously-broken user to a project. Confirm they persist after refresh.
  • Confirm the resolution was logged: grep "resolved userId=.*via email-address fallback" catalina.out
  • Confirm response JSON no longer includes that id in unresolvedUserIds.
  • Create a new user via UserCreate with email-only path; confirm USERS.UUID is a proper UUID.
  • Regress the non-buggy path: add a user with a real UUID to a project still works.
  • Project creation with mixed valid/broken user ids: valid and email-matched users are added; truly unresolvable ids appear in unresolvedUserIds.

JasonWildMe and others added 2 commits April 16, 2026 13:27
…ip (#1545)

UserCreate was calling new User(email), which resolves to User(String uuid)
and stored the email as the account's UUID primary key. Those accounts'
getUUID() returns an email, so the project autocomplete posts an
email-shaped id; ProjectUpdate/ProjectCreate then fall through to a
username lookup that fails, and silently report success.

- UserCreate: use new User(email, Util.generateUUID()) to stop creating
  more broken rows.
- User(String uuid): @deprecated — callers have misused it historically.
- ProjectUpdate/ProjectCreate: add a shared resolveUser() with an
  email-address fallback for legacy accounts, log every fallback hit,
  and surface unresolvedUserIds in the response so failures are visible.

Does not migrate existing bad rows — that follows in a separate ticket
with a JSP-driven, admin-auth'd, dry-run-capable data migration that also
reindexes affected encounter/occurrence docs in OpenSearch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex review on #1546 flagged a real bug: a non-UUID-shaped identifier
went to getUser(username) before the email fallback, so a legacy
email-as-UUID account could be mis-resolved to a different user whose
username happens to equal that email.

Put getUserByUUID first regardless of string shape — JDO application
identity does a literal PK lookup, so it resolves real UUIDs AND
email-as-UUID rows correctly, without risking the wrong match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.65%. Comparing base (cd7a80f) to head (a0ca664).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #1546    +/-   ##
========================================
  Coverage   51.65%   51.65%            
========================================
  Files         281      281            
  Lines       10309    10309            
  Branches     3195     3095   -100     
========================================
  Hits         5325     5325            
- Misses       4714     4720     +6     
+ Partials      270      264     -6     
Flag Coverage Δ
backend 51.65% <ø> (ø)
frontend 51.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JasonWildMe added a commit that referenced this pull request Apr 16, 2026
Codex review on #1546 flagged a real bug: a non-UUID-shaped identifier
went to getUser(username) before the email fallback, so a legacy
email-as-UUID account could be mis-resolved to a different user whose
username happens to equal that email.

Put getUserByUUID first regardless of string shape — JDO application
identity does a literal PK lookup, so it resolves real UUIDs AND
email-as-UUID rows correctly, without risking the wrong match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JasonWildMe JasonWildMe self-assigned this Apr 20, 2026
@naknomum naknomum added this to the 10.10.5 milestone May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACW - Can't save user to project

3 participants