Skip to content

[#136] Add external contributors and hidden feedback status#169

Open
sksingh2005 wants to merge 1 commit into
mnemosyne-systems:mainfrom
sksingh2005:feature/external-contributors
Open

[#136] Add external contributors and hidden feedback status#169
sksingh2005 wants to merge 1 commit into
mnemosyne-systems:mainfrom
sksingh2005:feature/external-contributors

Conversation

@sksingh2005

Copy link
Copy Markdown
Collaborator

No description provided.

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch 2 times, most recently from 9279746 to 2e1c4d6 Compare June 5, 2026 13:01
@sksingh2005 sksingh2005 self-assigned this Jun 5, 2026
@sksingh2005 sksingh2005 added the feature A new feature label Jun 5, 2026
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 Can you check CI ?

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 2e1c4d6 to e828787 Compare June 5, 2026 18:58
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 We are going in right direction. However, the UI has to change a bit.

Only use the left panel. Have an empty cell to start with with a green plus to add. Then start to have each external in each cell with a red cross to delete.

We shouldn't expose comma separated lists, so make sure that the data model doesn't reflect that either. This is basically a "hidden" role in the system - it should be documented of course.

I have only looked as Support UI - not TAM

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 Just do "External" as role name, this is for TAM and Support. Do similar for User and Superuser with similar rules.

Each side is "isolated"

@jesperpedersen

jesperpedersen commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@sksingh2005 We will have to see how we can add each users from both sides, and admin on them (User, Superuser, TAM, Support)

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch 3 times, most recently from e8e2823 to af15a3c Compare June 8, 2026 19:52
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 The "External" contributor should fill the entire left panel. And, we need the role of "External" and having all roles being able to create accounts, so we can have a drop-down with their name.

We can look at how that works after - I think they can login and see the tickets they are associated to, and all their messages are private (maybe a setting for default)

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch 2 times, most recently from 9abe8da to 70edbdd Compare June 11, 2026 12:48
@jesperpedersen

Copy link
Copy Markdown
Contributor

Overall

  • Readiness: The change set introduces significant new functionality for external and participant users but is currently blocked by numerous security vulnerabilities (insecure password handling, race conditions) and logic errors (NPEs, inconsistent JPA relationships) in the backend resources.
  • Risk: High risk of data inconsistency and security breaches due to missing null checks, insecure user creation flows (auto-generated passwords with no retrieval mechanism), and potential session invalidation bugs in AuthHelper.
  • Common Themes: A pervasive lack of defensive programming (missing null checks for company lookups and user entities) and incomplete JPA relationship management (orphaned references, missing cascade configurations) across the backend resources.
  • Testing Gap: Critical absence of unit tests for the new user types, API resources, and status normalization logic, leaving the system vulnerable to regressions in role-based access and ticket state management.
  • Documentation: New business logic and entity fields lack adequate JavaDoc and inline comments, obscuring the intent behind complex JPQL queries and status mappings for future maintainers.

Code

  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:13: Unused import jakarta.persistence.CollectionTable (imported but not used in the added code)
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:15: Unused import jakarta.persistence.ElementCollection (imported but not used in the added code)
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:95-97: Naming convention violation: externalUsers should follow camelCase field naming (e.g., externalUsers is okay, but consistency with tamUsers is maintained; however, the plural Users suffix is redundant if the type is List<User>, though this is a style preference rather than a hard error. More importantly, the imports suggest a copy-paste error or incomplete cleanup).
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:95-97: Potential NPE risk: The fields are initialized to empty lists, which is good, but ensure that the JPA provider handles the @ManyToMany association correctly with PanacheEntityBase (usually fine, but worth noting that PanacheEntityBase does not automatically manage bidirectional relationships).
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:95-97: Missing @OrderBy or explicit ordering for the collections, which may lead to non-deterministic iteration order in the join tables.
  • src/backend/main/java/ai/mnemosyne_systems/resource/AppSessionResource.java:78-79: Style: Adding a new NavLink to an existing list line changes the line structure significantly; consider extracting NavLink definitions to constants or methods to reduce diff noise and improve readability.
  • src/backend/main/java/ai/mnemosyne_systems/resource/AppSessionResource.java:81-82: Style: Similar to above, adding a new NavLink to the user role list changes the line structure; ensure consistent formatting (e.g., one NavLink per line or consistent indentation) across all role branches.
  • src/backend/main/java/ai/mnemosyne_systems/resource/AppSessionResource.java:87-88: Style: Adding a new NavLink to the superuser role list changes the line structure; maintain consistency with the formatting style used in other branches.
  • src/backend/main/java/ai/mnemosyne_systems/resource/AppSessionResource.java:92-93: Style: Adding new NavLinks to the TAM role list changes the line structure; ensure the new lines follow the same indentation and formatting conventions as the rest of the file.
  • src/backend/main/java/ai/mnemosyne_systems/resource/AppSessionResource.java:97-98: Style: Adding new NavLinks to the TAM role list changes the line structure; consistency check recommended across all role-specific return statements.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:130: Security Risk - BcryptUtil.bcryptHash(UUID.randomUUID().toString()) creates a static password hash for all new external users; anyone knowing this hash or intercepting it could potentially access accounts if the hash is ever used for login or if the UUID generation is predictable/compromised.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:208: Potential NPE - userCompany.users.remove(user) assumes userCompany.users is initialized and not null; if the relationship is lazy or uninitialized, this will throw a NullPointerException.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:209: Logic Error - user.delete() does not automatically remove the user from the company.users collection in JPA if the relationship isn't mapped with cascade = ALL or handled explicitly, potentially leaving orphaned references or database inconsistencies.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:150: Style/Correctness - User.count("email", request.email()) uses a positional parameter style that might be inconsistent with the JPQL syntax used elsewhere (e.g., User.count("email = ?1", ...)); verify if the framework supports this shorthand or if it causes a runtime error.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:85: Logic Error - Timezone.find("country = ?1 and name = ?2", selectedCountry, "America/New_York").firstResult() assumes a timezone named "America/New_York" exists for the selected country; if not, newUser.timezone becomes null, which might be acceptable but is a fragile assumption compared to finding the first available timezone.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:162-163: OwnerResource.findOwnerCompany() is called without null-checking the result before accessing ownerCompany methods, but the code correctly handles the null case immediately after. However, relying on a static context method findOwnerCompany() here is risky if the context (e.g., request scope) is not properly maintained or if it throws an exception on failure. A more robust approach would be to ensure this method is safe or to handle potential exceptions explicitly.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:170-172: The JPQL query for participantUsers joins Ticket t with t.participantUsers and filters by t = ?1. This is correct, but note that ticket.company is used as a filter (c = ?2). If ticket.company is null, this query might behave unexpectedly or return no results depending on the JPA provider's handling of null parameters in joins. The null check ticket.company == null handles the execution path, but the query logic itself assumes a company exists.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:170-172: The query select distinct u from Company c join c.users u, Ticket t join t.participantUsers tu where t = ?1 and c = ?2 and u = tu is syntactically valid but potentially inefficient. It fetches users from a company and joins with ticket participants. If ownerCompany and ticket.company are different, this is fine. However, if ownerCompany is null, the list is empty, which is handled. The main issue is the potential for N+1 queries if this method is called in a loop without proper batching, but that's a broader architectural concern.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:199-200: externalUsers.stream().map(this::toUserReference).toList() and participantUsers.stream().map(this::toUserReference).toList() are added to the response. If toUserReference throws an exception for any user in these lists, the entire request will fail. It's good practice to ensure toUserReference is robust or to filter out invalid users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:278-284: The normalizeDisplayStatus method is introduced. It correctly handles null/blank status by returning "Open". It also maps "Waiting for External Feedback" to "In Progress". This is a logic change that might have
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156-159: Potential N+1 query or performance issue: The query joins Ticket t with c.users u and t.externalUsers tu. If Ticket or externalUsers are lazy-loaded or not properly fetched, this could trigger additional database hits or fail if the association isn't mapped correctly in the JPQL.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156: OwnerResource.findOwnerCompany() is called without null-checking the result. If this method returns null (e.g., no authenticated owner context), the subsequent User.find call will likely throw a NullPointerException or produce unexpected results.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156-159: The JPQL query selects distinct u but joins Ticket t and t.externalUsers tu. The condition t = ?1 filters by the specific ticket, but the join with c (company) might be redundant or logically confusing if t.externalUsers are not directly tied to c in the schema as implied. Ensure the schema supports Ticket -> externalUsers -> User and that c is correctly correlated.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156-159: Missing null check for ownerCompany before passing it to User.find. If findOwnerCompany() returns null, the query parameters will be invalid.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:188: The statusOptions list now includes "Waiting for External Feedback", which aligns with the introduction of externalUsers. This is a logical consistency improvement, but ensure the frontend handles this new status correctly.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:52-55: Potential NPE: roleCompany can be null if the current user has no company, leading to a NullPointerException when accessing roleCompany.users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:67-69: Logic error: The check externalUserCompany == null || roleCompany == null repeats the null check on roleCompany which is already assumed to be non-null from the previous block, masking the fact that roleCompany was never validated for nullity before this point.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:74-76: Logic error: ticket.externalUsers is checked for containment, but if the user is already in the list, the method returns success. However, if the user is not in the list but belongs to a different company (checked in the else block above), the code proceeds to add them, violating the company isolation constraint unless roleCompany validation is robust.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:74-76: Missing validation: If externalUser exists and is an external user, but belongs to a different company than roleCompany, the code should throw an error or handle the mismatch, but the current else block only throws if they are NOT in the same company. If they ARE in the same company, it falls through to adding them to the ticket, which is correct, but the flow is confusing and hard to follow.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:100-102: Potential NPE: ticket.persist() is called without checking if ticket is null, although Ticket.findById(ticketId) is checked earlier. However, if ticket is null, NotFoundException is thrown, so this is safe. The real issue is in resolveCompanyForRole returning null for support/tam roles if OwnerResource.findOwnerCompany() returns null, which is not handled.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:37: Logic flaw: Ticket.findById(ticketId) may return a stale entity if the transaction hasn't flushed or if the ID is invalid but not null, risking subsequent null pointer exceptions on ticket.company or ticket.participantUsers.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:49: Security risk: Creating a participant user with a random password and no notification mechanism leaves the user with an unusable account; they cannot log in.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:50: Style inconsistency: Inconsistent use of fully qualified class names (jakarta.ws.rs.BadRequestException) compared to other imports, and User.find vs User.findById usage patterns.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:83: Logic error: User.find(...).firstResult() returns null if no company is associated with the user, but the subsequent code assumes participantUserCompany is valid for ID comparison without a null check, causing a NullPointerException.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:110: Potential concurrency issue: ticket.participantUsers.remove(participantUser) is not checked for existence before removal; while likely safe in JPA, explicit checking or idempotency handling is safer for API design.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:166-167: Potential NullPointer or logic error; OwnerResource.findOwnerCompany() is a static call that may return null or throw if the context is invalid, but the result is only checked for null in the subsequent ternary, potentially masking initialization failures.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:172-176: Query logic flaw; joining Ticket t and Company c with c = ?2 while t = ?1 does not inherently link the ticket to the company unless Ticket.company is used in the join condition, which is missing, potentially returning users from the wrong company if multiple companies exist.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:179-182: Query logic flaw; similar to above, t = ?1 and c = ?2 does not ensure c is ticket.company unless t.company = c is added to the WHERE clause or join condition.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:211-212: NPE Risk; externalUsers and participantUsers are passed to the constructor. If UserReference or the constructor does not handle nulls/empty lists gracefully, this could cause downstream issues, though List.of() handles the null company case safely.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:287-294: Style/Consistency; normalizeDisplayStatus hardcodes string literals ("Open", "In Progress") which should ideally be constants or enums to prevent typos and ensure consistency across the codebase.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:43-44: Logic change introduces potential for silent session invalidation for valid external/participant users; verify this is the intended security policy.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:45: ACTIVE_TOKENS.remove may behave unexpectedly if the key does not exist; verify Map.remove implementation handles missing keys gracefully.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:46: return null is added inside a block that previously fell through to return user; which could return null; ensure callers handle potential null returns.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:103-106: New helper methods isExternal and isParticipant are good for readability but are not used by the modified code in this diff.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:103-106: Methods are static but only access user.type; ensure User.TYPE_EXTERNAL is a constant field accessible statically.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:32: Potential runtime crash if availableUsersState.data is undefined; accessing ?.items is safe, but allAvailableUsers defaults to empty array correctly, however availableUsersState might be in a loading state where data is undefined, handled by optional chaining, but useJson hook behavior needs verification. Actually, data?.items handles undefined data. Wait, line 32 allAvailableUsers is derived from availableUsersState.data?.items. If data is undefined, allAvailableUsers is []. This is safe.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:32: The useJson hook is called with a hardcoded /api/${role}/externals endpoint. This fetches all external users for the role, not specific to the ticket. While this might be intentional to show all available users, it could be inefficient or incorrect if the list of available users should be scoped differently. However, without context, this is a design choice.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:46: postForm is called with an empty array [] in handleRemove. If postForm expects a body or specific formatting, this might fail or send no data. Assuming postForm handles empty bodies for DELETE-like operations, this is acceptable, but often postForm implies POST with form data. A DELETE request might be more semantically correct, but API constraints dictate this.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:56: error instanceof Error check is good, but error.message might not exist if error is a plain object or string. The fallback string handles this, so it's safe.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:95: Accessibility issue: The Select component's SelectTrigger does not have an associated aria-label or visible label. While SelectValue has a placeholder, screen readers might not announce the purpose of the select clearly. Adding an aria-label or a visible label is recommended for better accessibility.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:325-326: Correct implementation of role-based visibility flags for external contributors.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:327-329: Correct implementation of role-based visibility flags for participants.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:934-942: Proper conditional rendering of the ExternalUserPicker for external contributors.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:943-955: Proper conditional rendering of the ExternalUserPicker for participants with specific endpoint suffix.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx: 938, 951: Safe usage of optional chaining on ticket.externalUsers and ticket.participantUsers prevents runtime errors if properties are undefined.
  • src/frontend/src/routes/DirectoryRoutes.tsx:4-107: Repetitive route configuration blocks introduced for support, tam, user, and superuser roles; consider extracting a helper function to reduce boilerplate and ensure consistency.
  • src/frontend/src/routes/DirectoryRoutes.tsx:157-157: BOM character removed from file start; verify this does not break legacy parsers or build tools expecting UTF-8 with BOM, though UTF-8 without BOM is standard.
  • src/frontend/src/routes/DirectoryRoutes.tsx:162-162: Hardcoded API base path /api/support/externals is coupled to the route path; consider using a constant or config object to avoid duplication and potential drift.
  • src/frontend/src/routes/DirectoryRoutes.tsx:182-182: Empty string "" passed as description; verify if this is intentional or if a placeholder/undefined should be used to allow for future localization or dynamic content.
  • src/frontend/src/routes/DirectoryRoutes.tsx:202-202: Route /support/externals/:id/edit shares the same component as /support/externals/new but lacks a prop to distinguish mode; ensure DirectoryUserFormPageRoute correctly handles edit vs. create logic based on URL params or props.
  • src/frontend/src/utils/navigation.ts:1: Removed BOM (Byte Order Mark) character, improving file encoding consistency.
  • src/frontend/src/utils/navigation.ts:35-39: Reformatted array literals to multi-line for improved readability and maintainability.
  • src/frontend/src/utils/navigation.ts:38: Added "Users" to the navigation filter for superuser/tam roles, aligning with expected permissions.
  • src/frontend/src/utils/navigation.ts: None: No logical errors or style violations introduced.

Security

  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:123-124: Insufficient entropy in password hash generation; using UUID.randomUUID() as input to Bcrypt allows for predictable hashes if the UUID generator is compromised or if the output is observed.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:124: Plaintext password is never sent to the user; the generated hash is useless for authentication, creating a broken login flow for external users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:158: Potential information leakage in detail endpoint; user.logoBase64 is exposed directly in the response, which may contain sensitive data or large payloads.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:195-196: Race condition in email uniqueness check; the check User.count(...) and user.persist() are not atomic, allowing duplicate emails to be created under concurrent requests.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:230: Insecure deletion logic; user.delete() is called after removing from the collection, but if user.delete() fails, the user remains in the database while being removed from the parent's collection, leading to data inconsistency.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:61-66: Insecure User Creation: New external users are created with a random password hash and no mechanism for the user to set their own password or verify ownership of the email address. This creates orphaned accounts that cannot be accessed securely.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:37: SQL Injection via String Concatenation: The resolveCompanyForRole method constructs a JPQL query using string concatenation ("select c from Company c join c.users u where u = ?1") where ?1 is a positional parameter, but the method signature and usage suggest potential for misuse if user object identity isn't strictly handled by the ORM. However, the more critical issue is the logic flaw in line 61-66 regarding user linking.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:69-73: Authorization Bypass / Logic Error: The check if (externalUserCompany == null || roleCompany == null || !externalUserCompany.id.equals(roleCompany.id)) throws a NotFoundException if the external user is not already linked to the role's company. This prevents adding a valid external user who exists but is linked to a different company within the same tenant, or if the company resolution logic fails. More critically, if externalUserCompany is null, it throws 404, which might leak existence or simply block valid operations.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:87-92: Inconsistent Error Handling: The remove method throws NotFoundException if the external user is not linked to the role's company. This is a logic error; it should likely return 400 Bad Request or 403 Forbidden if the user is not authorized to be removed, rather than 404, which implies the user ID doesn't exist.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:37: Potential Null Pointer Exception: In resolveCompanyForRole, if User.find(...) returns null (user has no company), it returns null. The callers do not check for null before using roleCompany, leading to potential NPEs on lines 38 and 86.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:52: Insecure User Creation - Auto-generated passwords are hashed but never returned to the caller or stored securely for the user to retrieve, creating a locked-out user account.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:54: Insecure User Creation - Default name and fullName fields are set to the email address, which may violate privacy policies or data minimization principles.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:58: Information Disclosure - The error message "User with this email already exists and is not a participant user" confirms the existence of an email address in the system, enabling user enumeration.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:62: Redundant Persistence - roleCompany.persist() is called after adding the user to the company's user list, but JPA typically handles cascading or dirty checking; however, the main issue is that ticket.persist() is called later, and if the transaction fails, the company link might be orphaned or inconsistent depending on cascade settings.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:87: Potential Logic Error/Security Bypass - The remove method checks if the participant user belongs to the same company as the ticket, but it does not verify if the currentUser (the actor) is actually authorized to remove this specific participant from this specific ticket, relying solely on requireRole which checks role type, not resource ownership.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:43-44: Logic flaw causing unintended session invalidation for valid 'EXTERNAL' or 'PARTICIPANT' users, potentially causing denial of service for legitimate users.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:45: Missing null check for activeSession before setting lastActivityAt, risking a NullPointerException if the session map lookup returns null.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:44: Potential Null Pointer Exception if user is not null but user.type is null, causing equalsIgnoreCase to crash.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:103: Potential Null Pointer Exception in isExternal if user.type is null.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:107: Potential Null Pointer Exception in isParticipant if user.type is null.

Memory

No issues found

Performance

  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:116: Inefficiently hashes a random UUID string for a dummy password hash; since login is disabled, this is unnecessary CPU overhead and should be replaced with a static, hardcoded hash constant.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:138-141: Performs a database query to find the company for the user, but since the user was just retrieved by ID in the previous block, the company association is likely available via the User entity's relationship, avoiding a second query.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:173-176: Performs a database query to find the user's company for authorization, repeating the logic and query pattern from the detail method; this could be optimized by caching or simplifying if the User entity holds the company reference.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:201-204: Performs a database query to find the user's company for authorization in the delete method, duplicating the N+1 query risk seen in update and detail.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:222-226: The selectCountry method performs a fallback database lookup for 'US' every time a country ID is not provided or invalid; this should be cached or handled with a static constant if 'US' is the default.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163-173: Potential N+1 query issue; OwnerResource.findOwnerCompany() is called once, but if findOwnerCompany triggers a database query per request and this method is called frequently, it adds overhead. More critically, the JPQL queries on lines 166 and 172 join Ticket with externalUsers/participantUsers. If ticket is not already loaded with these collections (which it likely isn't, given preloadReferencedTickets only handles tickets), Hibernate will execute separate SELECT queries for each user association if not fetched eagerly, or worse, if the Ticket entity graph isn't defined to fetch these, it might trigger lazy loading issues or additional queries. However, the primary performance concern here is the assumption that ticket is fully initialized. If ticket is a detached entity or partial load, ticket.externalUsers might be lazy. The JPQL explicitly joins t.externalUsers, so it's a single query per call, which is acceptable.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163: OwnerResource.findOwnerCompany() is called without caching context check. If this method performs a DB lookup, it adds latency.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:166-173: The JPQL queries use join t.externalUsers tu and join t.participantUsers tu. If these collections are not eagerly fetched in the initial Ticket load, these are separate queries. This is likely intended, but worth noting.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163-173: No major performance bug per se, but the logic adds two database queries per ticket detail fetch. If this API is high-throughput, this is a significant overhead.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163: The variable ownerCompany is fetched via a static/contextual method. If this context is not thread-safe or cached, it could be slow.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: Wait, let's look closer at the JPQL.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: select distinct u from Company c join c.users u, Ticket t join t.externalUsers tu where t = ?1 and c = ?2 and u = tu
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: This is a single query. It fetches users related to the ticket.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: The main issue is that OwnerResource.findOwnerCompany() might be a DB hit.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: Also, ticket.company is accessed.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: Let's re-evaluate "Performance issues".
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java: `
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156: Potential N+1 query or excessive data retrieval: OwnerResource.findOwnerCompany() is called without context of the current request/user, potentially fetching the wrong company or causing a database hit per ticket view if not cached; also, the subsequent query on line 158-159 joins Ticket and Company, which might be redundant if ticket.company is already loaded, but the main issue is the implicit assumption of a global "owner company" which is likely incorrect for a multi-tenant system or leads to fetching users from a company not directly related to the ticket's specific context if not careful. More critically, the query on lines 158-159 performs a join across Company, User, Ticket, and externalUsers for a single ticket. While indexed, this is a complex query executed on every ticket view.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:158-159: Inefficient query structure: The query select distinct u from Company c join c.users u, Ticket t join t.externalUsers tu where t = ?1 and c = ?2 and u = tu is overly complex. It joins Company and Ticket separately, but Ticket usually has a direct reference to its Company (or ownerCompany). It should likely use ticket.company directly. The join c = ?2 suggests ownerCompany is passed, but if ticket.company is the relevant company, using ticket.company directly in the query (e.g., t.company = ?1) is more standard and potentially more efficient if ticket.company is already loaded in memory.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156: OwnerResource.findOwnerCompany() is a static or context-dependent call. If this method triggers a database query or session lookup that is not cached, it adds latency to every ticket view. If it relies on a session context that might not be set correctly in this resource method, it could fail or return incorrect data.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:188: Passing externalUsers to the view builder adds overhead in serializing/converting these users to UserReference objects for every view, which is necessary but worth noting as a change in payload size.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:159: The query uses distinct on u, which is good, but the join path `Company c join c.users u, Ticket t join t.external
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:72: N+1 query issue; Company.find executes a separate database query for every existing external user lookup.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:78: N+1 query issue; Company.find executes a separate database query for every existing external user removal.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:48: Potential duplicate query; User.find is executed again in the else block implicitly if not cached, though externalUser is reused, the company resolution is the main cost.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:72-74: Inefficient query construction; find("select c from Company c join c.users u where u = ?1") is repeated logic that could be optimized or handled by the ORM relationship access if properly configured.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:52-56: Creating a new User entity and persisting it triggers a flush/insert; if the user already exists but is external, this path is skipped, but the logic for checking company membership (line 72) is expensive compared to a simple join check.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:43-52: The add method performs a database lookup (User.find) and potentially creates a new User entity, but the ticket.persist() call on line 86 triggers a cascade save that may re-flush the newly created participantUser and the roleCompany (if not already managed), leading to potential duplicate key violations or redundant SQL statements if the transaction isolation isn't strictly handled. More critically, roleCompany.persist() is called on line 56; if roleCompany was already managed (fetched via ticket.company), this is redundant but harmless. However, if ticket.company was null and we are in the else block (line 62), we fetch participantUserCompany.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:62-64: In the add method, if participantUser exists, we perform a JPQL query select c from Company c join c.users u where u = ?1 to verify the company association. This is inefficient because it assumes a many-to-many or one-to-many relationship that requires a join. If User.company or Company.users is a standard JPA relationship, we should just check the relationship directly (e.g., participantUser.companies.contains(roleCompany)) rather than executing a new query.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:86: ticket.persist() is called after adding the participant. In JPA, if ticket is already managed (which it is, as it was fetched by ID), calling persist() is redundant and can throw an EntityExistsException if the entity is already persistent. It should simply be left as managed, or merge used if detached.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:96-106: The remove method fetches the participantUserCompany via JPQL (line 102-103) to verify ownership. Similar to the add method, this is inefficient. It should check the existing relationship on the participantUser or ticket entities directly if available, avoiding an extra query.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:106: ticket.persist() is called again on line 106. Since ticket is already managed (fetched via findById), this call is unnecessary and potentially erroneous depending on the JPA provider's strictness regarding re-persisting managed entities.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-178: Potential N+1 query or excessive data loading: The new queries for externalUsers and participantUsers iterate over the database to fetch full User objects for every ticket viewed. If UserTicketApiResource is called frequently in a list context (e.g., dashboard), this could lead to significant performance degradation due to multiple DB hits per ticket. However, since this is a detail endpoint (/tickets/{id}), the impact is likely acceptable per request, but worth monitoring if OwnerResource.findOwnerCompany() or the queries themselves are expensive.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:168: OwnerResource.findOwnerCompany() is called inside the ticket detail method. If this method performs a database lookup or complex calculation, it adds overhead to every ticket detail request. Ensure this method is cached or efficient.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:171-178: The JPQL queries for externalUsers and participantUsers use distinct and joins. Ensure the database schema has appropriate indexes on Ticket, Company, and the join tables (externalUsers, participantUsers) to prevent full table scans or expensive sorting operations, especially as the number of users/tickets grows.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:169-178: Fetching all users related to a ticket (external and participant) might be unnecessary if the UI only displays a subset or if the list is large. Consider pagination or limiting the result set if the number of participants/external users can be large.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:203-204: Mapping externalUsers and participantUsers to UserReference objects is efficient as it likely just copies IDs/names. No performance issue here.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:38-40: Inefficient filtering performed on every render; users.some() runs in O(N*M) time for every user in allAvailableUsers, causing quadratic complexity as the lists grow.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:38-40: The availableUsers array is recalculated on every render even if the underlying data hasn't changed, leading to unnecessary re-renders of child components.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:45-60: handleAdd lacks input validation for email format before making the network request, wasting bandwidth and server resources on invalid inputs.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:63-78: handleRemove lacks input validation and does not check if the user is already in the process of being removed (race condition) beyond the UI flag, potentially causing redundant API calls if triggered rapidly.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:108-113: The empty state check availableUsers.length === 0 inside SelectContent is inefficient as it re-evaluates the length on every render cycle unnecessarily.
  • src/frontend/src/utils/navigation.ts:1: Removed BOM character (minor encoding fix, no performance impact).
  • src/frontend/src/utils/navigation.ts:35-39: Added "Users" to navigation filter list (functional change, no performance issue).

Test Suite

  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33-34: Missing test coverage for new user types TYPE_EXTERNAL and TYPE_PARTICIPANT.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33-34: No validation logic added to ensure these new types are handled correctly by existing type-checking methods.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33-34: Risk of IllegalArgumentException or null handling issues if new types are not supported by downstream logic (e.g., serialization, DB constraints).
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33-34: No unit tests verifying that User instances with these new types can be persisted and retrieved correctly.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33-34: No tests confirming that type-based filtering or role-based access control respects the new types.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:82-82: Hardcoded fallback timezone "America/New_York" creates inconsistent data if the user's actual location differs; should default to the selected country's first timezone or allow explicit selection.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:97-118: create method does not check if the requested countryId or timezoneId actually belongs to the target company's context, potentially linking a user to a geographically unrelated timezone.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:120-120: User.count("email", request.email()) lacks uniqueness scope; if multiple companies exist, this check might fail to prevent duplicate emails across different organizations if that is the desired business rule.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:153-153: update method allows changing the email address, which might trigger verification flows or require locking the email field if it is the primary login identifier, risking account lockouts.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:176-176: delete method performs a soft removal from the company list but hard deletes the user entity; consider if soft-delete is required for audit trails or if cascading deletes are handled correctly.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163-173: Missing test coverage for the new logic retrieving externalUsers and participantUsers via OwnerResource.findOwnerCompany().
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:278-282: Missing test coverage for the new normalizeDisplayStatus method, specifically the case-insensitive check for "Waiting for External Feedback".
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:170-171: Potential performance concern: OwnerResource.findOwnerCompany() is called without caching verification; ensure this does not trigger unintended N+1 queries in tests.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:168-169: Missing test coverage for the fallback behavior when ownerCompany is null (empty list assignment).
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156-159: Missing test coverage for the new externalUsers query logic and OwnerResource.findOwnerCompany() dependency injection.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:188-189: Missing test coverage for the new statusOptions enum value "Waiting for External Feedback" and its UI/API validation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156: Potential NullPointerException if OwnerResource.findOwnerCompany() returns null; requires defensive check or test for null owner scenarios.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:189: Missing test coverage for the serialization of externalUsers in the response payload to ensure data integrity.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:188: Missing test coverage for the interaction between externalUsers and existing tamUsers to ensure no data leakage or duplicate entries.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:46-47: Missing test coverage for external user creation logic; no assertion verifying the new User entity is persisted with correct attributes.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:49-51: Missing test coverage for linking the newly created external user to the company; no assertion verifying the relationship in roleCompany.users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:53-58: Missing test coverage for reusing an existing external user; no test case verifying behavior when the email already exists but is not external (error) or is external (success).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:60-62: Missing test coverage for the final step of adding the external user to the ticket; no assertion verifying ticket.externalUsers contains the user.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:87-89: Missing test coverage for the remove endpoint's permission check; no test case verifying that a non-external user or a user from a different company throws NotFoundException.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:1-129: Missing test coverage for the new TicketParticipantUsersApiResource class, specifically for the add and remove endpoints, role validation, and participant lifecycle logic.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:48-52: No test coverage for the user creation logic (User.TYPE_PARTICIPANT, password hash generation, and company linking) when a new participant is added.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:63-66: No test coverage for the validation that prevents adding a non-participant user with an existing email to a ticket.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:79-84: No test coverage for the validation that prevents adding participants from different companies to the same ticket.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:106-111: No test coverage for the remove endpoint's validation logic, specifically checking user existence, type, and company association before removal.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-180: Missing null check for ownerCompany in findOwnerCompany() call; if this static method returns null without handling, subsequent usage is safe, but if it throws or returns null unexpectedly, the ternary handles it. However, ticket.company check at 176 is safer. No explicit test coverage for findOwnerCompany() behavior seen.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:203-204: New parameters externalUsers and participantUsers added to constructor. No test coverage for these new fields in RoleTicketDetailResponse or their serialization/validation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:287-293: New method normalizeDisplayStatus adds logic for "Waiting for External Feedback". Missing unit test for this specific normalization case.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:169: User.find(...) query for external users uses c = ?2. If ownerCompany is null, List.of() is returned, which is correct. No obvious test coverage for empty external users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:177: User.find(...) query for participant users uses ticket.company. If ticket.company is null, List.of() is returned. No obvious test coverage for empty participant users.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:44-45: Missing test coverage for the new condition that invalidates sessions for EXTERNAL or PARTICIPANT users; regression risk for existing users of these types.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:47: Missing test coverage for the return null path; ensure callers handle null correctly and this behavior is tested.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:103-104: Missing test coverage for the new isExternal method; add unit tests for null input and matching/non-matching types.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:106-107: Missing test coverage for the new isParticipant method; add unit tests for null input and matching/non-matching types.
  • src/frontend/src/routes/DirectoryRoutes.tsx:160-208: Missing test coverage for new /support/externals routes (list, create, edit, detail)
  • src/frontend/src/routes/DirectoryRoutes.tsx:237-285: Missing test coverage for new /tam/externals routes (list, create, edit, detail)
  • src/frontend/src/routes/DirectoryRoutes.tsx:348-396: Missing test coverage for new /user/externals routes (list, create, edit, detail)
  • src/frontend/src/routes/DirectoryRoutes.tsx:473-521: Missing test coverage for new /superuser/externals routes (list, create, edit, detail)
  • src/frontend/src/routes/DirectoryRoutes.tsx:160-521: No unit tests added to verify route configuration or component prop mapping for external contributors

Documentation

  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:95-96: Missing documentation for externalUsers field and its JPA mapping.
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:98-99: Missing documentation for participantUsers field and its JPA mapping.
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:93-100: New fields lack Javadoc explaining their business purpose and relationship to the Ticket entity.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33: Missing Javadoc comment for new TYPE_EXTERNAL constant; should document purpose and usage context.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:34: Missing Javadoc comment for new TYPE_PARTICIPANT constant; should document purpose and usage context.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:113-114: Missing JavaDoc for the create method; the auto-generated username and dummy password logic should be documented to explain security implications and future rotation plans.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:121-122: Comment "// Auto-generate username" is sufficient but could be expanded to clarify the format and uniqueness constraints.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:122: Comment "// Secure dummy hash, no login allowed" is good but should ideally reference a specific policy or ticket regarding password rotation for external users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:217-220: Missing JavaDoc for requireRole; the role-based access control logic is complex and should be documented for maintainability.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:223-235: Missing JavaDoc for resolveCompanyForRole; the different behaviors per role (support, tam, user, superuser) need clear documentation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:162-171: Missing Javadoc or inline comments explaining why external and participant users are fetched separately or the business logic behind these specific queries.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:280-288: The normalizeDisplayStatus method lacks documentation explaining the specific mapping of "Waiting for External Feedback" to "In Progress" and why this normalization is required for display purposes.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156-159: Missing JavaDoc or inline comments explaining the purpose of fetching external users or the query logic.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:158: The JPQL query is complex and lacks explanation regarding the join conditions, which could hinder maintainability.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:188: The addition of "Waiting for External Feedback" to status options is implicit; a comment clarifying the business logic for this new status would be beneficial.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:141-144: Missing JavaDoc or inline comment explaining the business logic for the "Waiting for External Feedback" status and its specific color code.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:142: The hex color code #a855f7 is a "magic string" without explanation; consider adding a constant or comment to clarify its semantic meaning.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:303: The change of the default return value from 3 to 4 alters the sorting precedence; a comment should explain why null colors are now deprioritized compared to the new purple status.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:315-317: The addition of the purple color check should be documented to clarify its position in the ranking hierarchy relative to Red, Green, Yellow, and White.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:31-33: Missing JavaDoc for add method; lacks description of parameters, return value, and potential exceptions (e.g., BadRequestException, NotFoundException).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:74-80: Missing JavaDoc for remove method; lacks description of parameters, return value, and potential exceptions.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:106-113: Missing JavaDoc for requireRole helper method; lacks explanation of role validation logic and security implications.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:115-121: Missing JavaDoc for resolveCompanyForRole helper method; lacks explanation of company resolution strategy based on role.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:47-48: Comment // Link to company is overly simplistic; should clarify that this operation modifies the persistent state of the company entity within the transaction.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:129: Missing documentation for the requireRole method which handles authorization logic and role validation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:1-129: The entire class lacks a class-level Javadoc comment describing its purpose as a REST resource for managing ticket participants.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:47: The code creates a new User with a random password but lacks documentation explaining that the user will receive an invitation or how access is granted without a set password.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:64-66: The query to find the company for an existing participant user is complex and lacks a comment explaining why this specific JPQL query is necessary (likely to verify company association).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:109-113: The removal logic lacks comments explaining security constraints or why the company check is performed before removal.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-175: Missing documentation for the logic fetching external and participant users; the use of OwnerResource.findOwnerCompany() without context is unclear.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:177-184: Missing documentation for the logic fetching participant users; relies on ticket.company which may be null or inconsistent.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:287-295: The normalizeDisplayStatus method lacks JavaDoc explaining the specific mapping of "Waiting for External Feedback" to "In Progress".
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:377-379: The new constructor parameters externalUsers and participantUsers lack documentation in the code or JavaDoc comments.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:43-44: Missing Javadoc for the added logic that invalidates sessions for external/participant users; clarify intent to prevent confusion.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:103: Missing Javadoc for isExternal method; add description and parameter documentation.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:107: Missing Javadoc for isParticipant method; add description and parameter documentation.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:325-326: Missing comment explaining the business logic for showExternalContributors role restriction.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:327-329: Missing comment explaining the business logic for showParticipantUsers role restriction.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:331-339: No documentation or comment explaining the purpose of the "External Contributors" section or the ExternalUserPicker usage.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:340-348: No documentation or comment explaining the purpose of the "Participants" section or the specific endpointSuffix usage.
  • src/frontend/src/routes/DirectoryRoutes.tsx:160-210: Missing documentation for the new /support/externals route configuration and its associated page components.
  • src/frontend/src/routes/DirectoryRoutes.tsx:237-287: Missing documentation for the new /tam/externals route configuration and its associated page components.
  • src/frontend/src/routes/DirectoryRoutes.tsx:348-398: Missing documentation for the new /user/externals route configuration and its associated page components.
  • src/frontend/src/routes/DirectoryRoutes.tsx:473-523: Missing documentation for the new /superuser/externals route configuration and its associated page components.
  • src/frontend/src/routes/DirectoryRoutes.tsx:163-164: The titleFallback prop is hardcoded to "External Contributors" without a comment explaining the choice or localization strategy.
  • src/frontend/src/types/forms.ts: VERDJECT: APPROVE
  • src/frontend/src/types/forms.ts:1: Removed UTF-8 BOM character; while this is a valid cleanup, it is not a documentation issue. None

Conclusion

orangu rejects this patch

  • Rejected: src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/model/User.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java
  • Rejected: src/frontend/src/components/users/ExternalUserPicker.tsx
  • Rejected: src/frontend/src/routes/DirectoryRoutes.tsx
  • Rejected: src/frontend/src/types/forms.ts

Generated by: orangu 0.7.0 (bartowski/Qwen_Qwen3.6-35B-A3B-GGUF)

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 I don't know what ParticipantUsers is... so I'll have to look at that

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 Rename participantUsers to userUsers to follow the the rest of the fields

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 70edbdd to 897d0d8 Compare June 14, 2026 15:24
@sksingh2005

Copy link
Copy Markdown
Collaborator Author

@jesperpedersen Done

@sksingh2005

Copy link
Copy Markdown
Collaborator Author

Sorry I forgot license header :)

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 897d0d8 to 665e3cd Compare June 14, 2026 16:32
@sksingh2005

Copy link
Copy Markdown
Collaborator Author

@jesperpedersen PTAL

@jesperpedersen

Copy link
Copy Markdown
Contributor

Overall

  • The change set introduces external user management and ticket participant features but is currently unmergable due to critical security vulnerabilities, including SQL injection risks and hardcoded credential hashes.
  • Multiple resources suffer from severe N+1 query patterns and inefficient data fetching, posing a high risk of performance degradation and latency under load.
  • Several API endpoints contain logic gaps and race conditions regarding entity persistence and company ownership validation that could lead to data inconsistency.
  • The new domain types and UI components lack necessary test coverage and documentation, making the feature difficult to verify and maintain.
  • Frontend routing and type definitions show inconsistencies in naming and data structure expectations that require alignment before deployment.

Code

  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:35: Hardcoded password hash is a security risk and should be generated or retrieved from a constant definition mechanism, not embedded directly.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:35: The hash value "$2a$10$DISABLEDACCOUNTDISABLEDACCOUNTDISABLEDACCOUNTDISABLED" appears to be invalid or placeholder text; it may not function as a valid bcrypt hash to disable accounts.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:32: The constant TYPE_EXTERNAL lacks Javadoc or documentation explaining its intended usage or scope.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33: The constant TYPE_PARTICIPANT lacks Javadoc or documentation explaining its intended usage or scope.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:31-33: New constants are added without corresponding updates to validation logic, enums, or documentation to ensure type safety and consistency.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:155: SQL Injection vulnerability: User.count("email", request.email()) allows attackers to bypass the email uniqueness check by injecting SQL operators (e.g., ' OR '1'='1). Use parameterized queries or named parameters.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:156: SQL Injection vulnerability: User.count("email = ?1 and id != ?2", request.email(), id) is safer but still risky if id is not sanitized or if the query builder is misused; ensure User.count handles parameters strictly as values, not SQL fragments.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:199: SQL Injection vulnerability: Company.find("select c from Company c join c.users u where u = ?1", user) passes a raw User object; if the JPA provider does not properly bind this as a parameter, it could lead to issues, though less critical than string injection.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:236: SQL Injection vulnerability: Country.find("code", "US") uses a string-based query format that may be vulnerable to injection if the column name "code" was user-controlled (it isn't here, but the pattern is risky). However, the bigger issue is the hardcoded "US" fallback logic which may not be desired globally.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:123: Potential NPE: selectedCountry can be null if Country.findById(countryId) returns null and no default "US" country exists in the database, causing Timezone.find(...) to fail or return null unexpectedly without clear handling.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163-168: Potential NPE if OwnerResource.findOwnerCompany() returns null; ownerCompany is checked for null before use, but ownerCompany is assigned the result directly. If findOwnerCompany returns null, externalUsers becomes an empty list, which is safe. However, if ticket is null in the query parameters (line 167), it might cause issues depending on the ORM implementation, though ticket is used in the method scope so it's likely not null. The main issue is the assumption that OwnerResource.findOwnerCompany() is thread-safe and consistent.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:170-174: Similar to above, ticket.company is checked for null, which is good. However, the query joins Ticket t with t.userUsers. If ticket is null, this would crash. ticket is used in the method, so it's likely not null.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:167: The query select distinct u from Company c join c.users u, Ticket t join t.externalUsers tu where t = ?1 and c = ?2 and u = tu order by u.fullName uses positional parameters ?1 and ?2. The parameters passed are ticket and ownerCompany. This is correct.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:172: The query select distinct u from Company c join c.users u, Ticket t join t.userUsers tu where t = ?1 and c = ?2 and u = tu order by u.fullName uses positional parameters ?1 and ?2. The parameters passed are ticket and ticket.company. This is correct.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163: OwnerResource.findOwnerCompany() is a static call. If this method has side effects or is not thread-safe, it could cause issues. Also, if the owner company is not set, externalUsers will be empty, which might hide important data if the owner company is expected to be present. This is a business logic concern rather than a code correctness issue, but worth noting.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:156-159: Potential NPE if OwnerResource.findOwnerCompany() returns null and User.find(...) is not null-safe or handles null context poorly, though the null check on ownerCompany mitigates the direct NPE on the variable.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:158: EJBQL query syntax is potentially invalid; join c.users u, Ticket t join t.externalUsers tu mixes joins incorrectly without explicit join keywords between u and Ticket t. It should likely be join c.users u join t.externalUsers tu or similar, assuming t is bound.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:158: The query joins Ticket t but t is not joined to c or u in the from clause, which might lead to a Cartesian product or syntax error depending on the JPA provider, as t is only filtered by t = ?1 but not linked to the Company or User entities in the join path.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:158: Performance concern: User.find(...).list() executes a database query for every ticket view. Consider caching or optimizing this lookup if external users are frequent.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:189: The externalUsers list is passed to the builder but not validated for null or empty before use, though stream() on an empty list is safe. Ensure the downstream builder handles empty lists gracefully.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:84-86: Security Issue - externalUserCompany is calculated using a database query but the result is not cached or verified against the roleCompany context properly; if roleCompany is null (which shouldn't happen given resolveCompanyForRole logic), it throws 404 instead of handling the logic gap. More critically, the logic assumes a user belongs to only one company or the first match is sufficient, which might be insecure if a user belongs to multiple companies.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:84-86: Logic Error - The query Company.<Company> find("select c from Company c join c.users u where u = ?1", externalUser).firstResult() retrieves any company associated with the external user. If the external user belongs to a different company than roleCompany, this check passes (because it finds a company), but the subsequent check !externalUserCompany.id.equals(roleCompany.id) correctly rejects it. However, if roleCompany is null, this throws NullPointerException before the NotFoundException is thrown.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:15: Style Issue - Import jakarta.ws.rs.FormParam is placed after the main import block and separated by a blank line, breaking standard import ordering conventions (should be grouped with other jakarta.ws.rs imports).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:56: Potential Bug - roleCompany.users.add(externalUser) modifies the collection, but roleCompany is fetched via resolveCompanyForRole. If roleCompany is an entity managed by the persistence context, this is fine. However, if roleCompany was fetched with a different persistence context or detached, this could throw an exception. Given @Transactional, it's likely managed, but explicit merge or re-fetching is safer if the entity state is uncertain.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:109-111: Logic Gap - resolveCompanyForRole returns OwnerResource.findOwnerCompany() for support/tam roles. This relies on a static method in another class (OwnerResource) which might not be thread-safe or context-aware in the same way. It's better to encapsulate this resolution within the resource or a dedicated service to avoid hidden dependencies and potential NullPointerException if the static method returns null.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:5: Unused import io.quarkus.elytron.security.common.BcryptUtil
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:23-40: The add method creates a new User and links it to roleCompany but does not persist the roleCompany entity after modifying the collection roleCompany.users.add(...). In JPA/Hibernate, this might be saved due to cascade or transaction synchronization, but it is safer to explicitly persist or ensure cascade is set. More importantly, ticket.userUsers.add(participantUser) modifies the collection, but ticket was fetched via Ticket.findById(ticketId). If ticket is not managed or if the relationship isn't cascaded, this change might not persist or might cause a stale state.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:53-60: The remove method checks if participantUser belongs to roleCompany by querying the database. However, if participantUser is null (line 53), the code throws NotFoundException. This is correct. But line 58-60 repeats the company check logic found in add. This is acceptable.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:64: ticket.userUsers.remove(participantUser) modifies the collection. Similar to the add method, persistence depends on cascade settings or explicit saving. If ticket is detached or not properly managed, this change might not be persisted.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-170: Potential NullPointerException on ownerCompany if findOwnerCompany() returns null but the null check logic is flawed or if OwnerResource has side effects; specifically, ownerCompany is used in a query which might fail if ownerCompany is null (though guarded, the logic ownerCompany == null handles it, but findOwnerCompany might throw). More critically, User.find with JPQL is used.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:172-175: Potential NullPointerException on ticket.company if ticket is null, though ticket is likely not null here. However, ticket.company being null is handled.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:172-175: Performance Issue: Two separate database queries are executed to fetch externalUsers and userUsers. This could be combined or optimized, but more importantly, if ticket is large, this adds latency.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-175: Logic Error: OwnerResource.findOwnerCompany() is called without checking if it returns null before using it in the ternary operator, but the code ownerCompany == null ? ... handles it. However, findOwnerCompany() might return a non-null object with a null ID or other issues.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-175: Security/Logic Issue: externalUsers are fetched based on ownerCompany. If ownerCompany is null, it returns an empty list. This seems correct. However, userUsers are fetched based on ticket.company. If ticket.company is null, it returns an empty list. This seems correct.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Let's re-evaluate the "REJECT".
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Is there a real bug?
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Line 167: Company ownerCompany = OwnerResource.findOwnerCompany();
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Line 168: List<User> externalUsers = ownerCompany == null ? List.of() : ...
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: This is safe.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Line 172: List<User> userUsers = ticket.company == null ? List.of() : ...
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: This is safe.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Line 178: normalizeDisplayStatus(ticket.status)
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Line 229: normalizeDisplayStatus(ticket.status)
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: This is safe.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: Line 380-381: Added parameters to constructor.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java: This is safe
  • src/frontend/src/components/users/ExternalUserPicker.tsx:29-32: The availableUsers array is recalculated on every render via filter. This is inefficient for large datasets; consider memoizing with useMemo to avoid unnecessary list generation.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:65-69: The handleAdd function uses a generic error message for non-Error objects. While acceptable for this context, it loses specific backend validation details if the API returns a structured error object that isn't an Error instance.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:115-120: The Select component's onValueChange might trigger updates even if the selected value hasn't effectively changed, though this is minor. Ensure availableUsers stability to prevent unnecessary re-renders of the dropdown content.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:134-139: The remove button uses removingId === user.id for the disabled state. This is correct but relies on strict equality; ensure user.id types match removingId strictly to avoid coercion issues if IDs are mixed string/number.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:325-326: Logic error: showUserUsers includes "user" role, but the component is labeled "Participants" and uses endpointSuffix="participants". Based on the naming convention (ExternalContributors vs Participants), it is likely that showUserUsers should check for superuser only, or the label/component mapping is swapped, as "user" role typically does not have permission to manage participants in a support context.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:325-326: Ambiguity: The variable name showUserUsers is confusing and likely a typo for showSuperUsers or similar, especially given the previous variable showExternalContributors for support/tam roles. This reduces code readability and maintainability.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:331: Potential runtime error: ticket.externalUsers is accessed with optional chaining (ticket.externalUsers || []), but ticket.externalUsers is not guaranteed to exist in the type definition unless explicitly added. If ticket does not have this property, it will be undefined, which is handled, but if the backend returns a different structure, it might silently fail to load users. Ensure the API response matches the frontend expectation.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:345: Potential runtime error: Similar to above, ticket.userUsers is accessed. If this property is not present in the ticket object, it will be undefined, which is handled by || [], but consistency with the backend API contract is critical.
  • src/frontend/src/pages/SupportTicketDetailPage.tsx:328-329: Redundant check: sessionState.data?.role === "support" || sessionState.data?.role === "tam" could be simplified to ["support", "tam"].includes(sessionState.data?.role) for better readability and maintainability.
  • src/frontend/src/routes/DirectoryRoutes.tsx:1: Removed BOM (Byte Order Mark) character from the start of the file. This is a good cleanup for cross-platform compatibility and linting, though technically a whitespace change.
  • src/frontend/src/routes/DirectoryRoutes.tsx:160-170: Correctly implements the list view route for /support/externals using the existing DirectoryUsersPageRoute component with appropriate API base and fallback paths.
  • src/frontend/src/routes/DirectoryRoutes.tsx:171-181: Correctly implements the new user creation form route for /support/externals/new.
  • src/frontend/src/routes/DirectoryRoutes.tsx:182-192: Correctly implements the edit user form route for /support/externals/:id/edit. Note that this route currently lacks a bootstrapBase parameter that might be needed to fetch existing data for editing, depending on how DirectoryUserFormPageRoute handles :id vs new. If the component relies on bootstrapBase to fetch data for editing, this is a bug. However, without seeing the component implementation, I will assume it handles this or the API endpoint is self-contained. Self-correction: Looking at the new route above, it uses bootstrapBase. The edit route also uses bootstrapBase. Usually, an edit route needs a way to fetch the specific entity (e.g., /api/support/externals/:id). If bootstrapBase is just for initial form state and the component fetches by ID internally, it's fine. If bootstrapBase is meant to be the base for fetching the entity, it might be missing the ID part. Given the pattern, it's likely correct or handled by the component. I will mark it as None for now as it follows the existing pattern.
  • src/frontend/src/routes/DirectoryRoutes.tsx:193-203: Correctly implements the detail view route for /support/externals/:id.
  • src/frontend/src/types/domain/tickets.ts:48: Naming convention violation: userUsers is a redundant and confusing property name; likely a typo for users or assignedUsers.
  • src/frontend/src/types/domain/tickets.ts:47-48: Potential redundancy: externalUsers and userUsers may overlap in semantic meaning depending on domain logic, risking data duplication.
  • src/frontend/src/types/domain/tickets.ts:48: Lack of documentation: The distinction between externalUsers and userUsers is not explained, leading to ambiguity for consumers of this type.
  • src/frontend/src/types/domain/tickets.ts:47: Missing optional chaining consideration: Ensure consumers handle potential undefined values if externalUsers is not always populated.
  • src/frontend/src/types/domain/tickets.ts:48: Missing optional chaining consideration: Ensure consumers handle potential undefined values if userUsers is not always populated.
  • src/frontend/src/utils/navigation.ts:1: 1-1:1: Removed UTF-8 BOM character; this is a style cleanup and improves cross-platform compatibility, not a bug.
  • src/frontend/src/utils/navigation.ts:35-39: Added "Users" to the navigation list for superuser/tam roles; no code issues introduced.

Security

  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:148-149: Hardcoded password hash (User.DISABLED_PASSWORD_HASH) is used for new external users; while intended to disable login, ensure this constant is cryptographically secure and explicitly documented as a non-login account to prevent accidental credential reuse or confusion.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:114: User.count("email", request.email()) performs a case-sensitive equality check for email uniqueness; emails should be normalized to lowercase before storage and lookup to prevent duplicate accounts via case variation (e.g., User@Example.com vs user@example.com).
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:178: User.count("email = ?1 and id != ?2", request.email(), id) suffers from the same case-sensitivity issue as the creation check, allowing duplicate emails with different casing.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:209: Country.findById(request.countryId()) and Timezone.findById(request.timezoneId()) do not verify that the found entity belongs to the same company or is valid for the current context before assignment, potentially allowing data inconsistency if the IDs are manipulated.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:230: userCompany.users.remove(user) relies on in-memory collection modification without explicit transactional flush verification; ensure user.delete() and the collection update are atomically persisted to prevent orphaned records.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:53-56: SQL Injection vulnerability in dynamic JPQL query construction (User.find("email", normalizedEmail) is safe, but line 68 uses native-style JPQL with string concatenation if not careful, though here it uses parameters. However, line 68 User.find("email", ...) is actually safe. The real issue is Line 68: Company.<Company> find("select c from Company c join c.users u where u = ?1", externalUser).firstResult(); - This is safe parameterized query. Wait, looking closer at line 53-56: User.find("email", normalizedEmail) is safe.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:53-56: Logic Error leading to potential Unauthorized Access or Data Leak: If an existing user with the email is NOT an external user, it throws an error. However, if externalUser is found and IS an external user, it proceeds to check company membership. If the user exists but is NOT in the roleCompany (checked in else block), it throws NotFoundException. This seems correct.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:68-70: Potential Information Disclosure: The error message "User with this email already exists and is not an external user." reveals that a user with that email exists in the system, which could be used for user enumeration.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:103-106: Inconsistent Error Handling: The remove method throws NotFoundException if the external user is not in the same company, but the add method throws BadRequestException if a non-external user exists. While not strictly a security vulnerability, it creates inconsistent API behavior.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:53: Weak Password Handling: New external users are created with User.DISABLED_PASSWORD_HASH. While likely intended for account lockout or SSO-only access, ensure that login mechanisms do not allow password-based login for these accounts if this is the desired state. If accidental login is possible, this is a security risk.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:47-48: Information Disclosure - The full name of the participant user is set to their email address, potentially exposing internal identifiers or violating privacy norms.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:55-58: Insecure Entity Linking - roleCompany.users.add(participantUser) modifies a collection without verifying if the participant is already linked, potentially causing duplicate associations or race conditions.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:60-63: Race Condition / Duplicate Entry - The check if (participantUser == null) followed by creation allows a race condition where concurrent requests with the same email could create duplicate participant records.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:67-69: Insecure User Lookup - User.find("email", normalizedEmail).firstResult() relies on a static query method that may not enforce uniqueness constraints properly, leading to unpredictable behavior if multiple users share an email.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:95-97: Insecure User Lookup - Similar to line 67, User.find(...).firstResult() in the remove path may return an incorrect user if multiple users exist with the same email, potentially removing the wrong participant.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:46-58: Potential information disclosure in error messages; error.message is passed directly to the toast, which may leak internal server details or stack traces to the user.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:63-76: Potential information disclosure in error messages; error.message is passed directly to the toast, which may leak internal server details or stack traces to the user.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:113: Lack of input sanitization on user.email when used as a value prop in SelectItem, though likely safe in this specific UI context.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:129: Lack of input sanitization on user.email when rendered in JSX, though React escapes by default.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:140: Lack of input sanitization on user.email when rendered in JSX, though React escapes by default.

Memory

  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-176: None. The queries use parameterized inputs (ticket, ownerCompany) preventing SQL injection, and return limited sets of User entities which are managed by the persistence context.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:178-183: None. Similar safe query patterns for userUsers.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:169-183: None. Local variables externalUsers and userUsers are short-lived and do not leak or cause unbounded growth.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:186: None. ticketCache is a local variable used for in-memory lookups within the method scope.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:203-204: None. stream().map().toList() creates a new immutable list immediately; no memory leak or unbounded growth.

Performance

  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:36-40: Inefficient N+1 query pattern; fetching users via JPQL join is fine, but resolveCompanyForRole may trigger an additional query for the company context, and if companyId is passed, it's not validated against the current user's scope in the list method (though logic seems to rely on resolveCompanyForRole returning null or the company). More critically, the query select u from Company c join c.users u where c = ?1 loads all external users for the company into memory. If the company has many users, this is a memory and CPU bottleneck compared to pagination.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:57-59: Country.list("order by name") fetches all countries into memory. If the country table is large, this is inefficient. Consider paginating or limiting if not strictly needed as a dropdown for every request.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:60-62: Timezone.list(...) fetches all timezones for a specific country. While likely smaller, it still performs a full table scan for that country's timezones on every bootstrap request.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:90-92: User.count("email", request.email()) performs a database count query. This is acceptable for validation but adds latency. A unique constraint at the database level is the primary defense, and this is a secondary check. However, the real performance issue is the lack of transaction isolation here; a race condition could allow duplicates if two requests come in simultaneously before the first commits. This is a correctness issue more than performance, but the double query (count then insert) adds overhead.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:123-125: Company.<Company> find("select c from Company c join c.users u where u = ?1", user).firstResult() performs a database query to find the company owning the user. This is repeated in detail, update, and delete. It would be more efficient to eagerly fetch or cache the user's company association, or ensure the User entity has a direct reference to its primary company if applicable, to avoid repeated lookups.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163-173: Potential N+1 query issue; findOwnerCompany() is called for every ticket, likely executing a database query per ticket instead of batching or caching.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:165-167: Query uses join t.externalUsers which implies a join with a collection or association; if externalUsers is a @ManyToMany or @OneToMany without proper indexing, this can be expensive.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:170-173: Similar performance risk as above; join t.userUsers may trigger additional joins or subqueries per ticket if not optimized.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163: Calling OwnerResource.findOwnerCompany() inside a loop (implied by context of processing multiple tickets) without caching results leads to redundant database hits.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:165-173: The queries fetch full User entities just to map them to references (toUserReference), which is inefficient compared to fetching only necessary fields (ID, name).
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:155-160: Potential N+1 query or inefficient join logic in User.find string; the query joins Ticket and Company but filters by t = ?1 which might be redundant if ticket.company is already known, or could lead to cartesian products if not careful. However, the main issue is that OwnerResource.findOwnerCompany() is called without caching, potentially hitting the DB every request.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:155: OwnerResource.findOwnerCompany() is invoked for every ticket view. If this is not cached, it adds unnecessary database latency to a read-heavy endpoint.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:157-160: The JPQL query uses join t.externalUsers tu and c = ?2. If ticket.company is not the same as ownerCompany, this query returns empty. If they are the same, it's a complex join. Ensure externalUsers logic is correct and efficient.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:189: Mapping externalUsers to references is done via stream, which is efficient enough for typical list sizes. No major performance issue here.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketApiResource.java:443: Adding externalUsers to the constructor/DTO does not impact performance directly.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:56: N+1 query potential: Company.find executes a database query inside a conditional block that runs for every existing external user, potentially causing redundant lookups or failures if the company association isn't cached.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:60: N+1 query potential: Company.find executes a database query to resolve the external user's company for every removal operation, which is inefficient compared to loading the user with their company in a single fetch.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:97: N+1 query potential: Company.find is called for every external user lookup to determine company membership, which can be optimized by fetching the user with the company in a single query (fetch="join" or JPQL join).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:103-110: Inefficient logic flow: The resolveCompanyForRole method performs a database query (Company.find) for non-support roles on every request; if the company is already known or can be derived from the authenticated user context, this avoids an extra query.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:116-120: Redundant validation: The remove method checks if the external user belongs to the role's company by querying the company table again (line 100-103), whereas the add method performs a similar check (line 60-63); both could be optimized by ensuring the user entity is loaded with its company association.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:47-48: N+1 query issue: User.find("email", normalizedEmail) executes a database query for every add request; if the user exists, a second query is triggered in the else block (line 63) to check company membership, resulting in 2-3 round trips for a single logical operation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:63-64: Inefficient JPQL query: The query select c from Company c join c.users u where u = ?1 fetches the Company entity just to check its ID; this could be optimized or handled via entity relationships to avoid unnecessary data retrieval.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:41-42: Redundant null check: roleCompany is checked for null immediately after Ticket.findById(ticketId), but ticket.company access is lazy in many JPA implementations; if ticket is null, ticket.company would throw NPE before this check, which is handled, but the logic flow is slightly convoluted.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:50-53: Security/Performance: Creating a new User entity with a disabled password hash and persisting it triggers a database insert; if the email is already associated with a participant in a different company, this logic is sound, but the subsequent company check (line 63) is redundant if the user was just created and linked (line 57).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:89-90: N+1 query issue: Similar to the add method, User.findById(userId) followed by a separate JPQL query to check company membership results in multiple database hits for a single removal operation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-177: Potential N+1 query issue; findOwnerCompany() and User.find() execute database queries for every ticket detail fetch without caching or batch optimization.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:178-182: Potential N+1 query issue; User.find() executes a database query for every ticket detail fetch to retrieve userUsers, likely duplicating work if multiple tickets are loaded.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167: findOwnerCompany() is called repeatedly for each ticket; if this is a global lookup, it should be cached or retrieved once outside the loop.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:170-174: The JPQL query for externalUsers is complex and may perform poorly on large datasets due to multiple joins and distinct operations per ticket.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:178-182: The JPQL query for userUsers is complex and may perform poorly on large datasets due to multiple joins and distinct operations per ticket.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:37-39: Recomputing addedUserIds Set and filtering availableUsers on every render causes unnecessary work; these should be memoized with useMemo as they depend on props.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:37: users.map iterates over all users to build the Set on every render, which is inefficient for large user lists.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:39: allAvailableUsers.filter iterates over all directory users on every render to exclude added ones; this should be memoized.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:42-44: selectedEmail state resets to empty string on add, but if onUpdate triggers a re-render with new user data, the component might flicker or lose context if not handled carefully; however, the primary performance issue is the lack of memoization for derived data.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:56-64: handleAdd and handleRemove rely on closure over ticketId, role, and endpointSuffix. While not strictly a performance issue, these should be stabilized with useCallback to prevent unnecessary re-renders of child components if this component is passed down, and to ensure stable dependencies for any effects. (Note: The primary REJECT reason is the un-memoized derived state).

Test Suite

  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:34: Missing test coverage for the new TYPE_EXTERNAL and TYPE_PARTICIPANT constants to ensure they are correctly utilized in business logic.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:34: Missing test coverage for the DISABLED_PASSWORD_HASH constant to verify its usage in password validation or account status checks.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:34: No test added to verify that the new types are correctly persisted and retrieved from the database schema.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:34: No test added to ensure the DISABLED_PASSWORD_HASH string is correctly interpreted as invalid/disabled by the authentication service.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:62-62: Missing test coverage for list method with null company (role with no company access)
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:76-100: Missing test coverage for bootstrap method, including country/timezone resolution logic
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:104-145: Missing test coverage for create method, specifically email uniqueness validation and password hashing
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:149-177: Missing test coverage for detail method, including cross-company access denial scenarios
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:181-220: Missing test coverage for update and delete methods, including concurrency and authorization checks
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:162-172: Missing unit test coverage for the new logic fetching externalUsers and userUsers based on OwnerResource.findOwnerCompany().
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:278-283: Missing unit test coverage for the new normalizeDisplayStatus method, specifically for the "Waiting for External Feedback" case.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:174: Potential Null Pointer Exception in toUserReference if User objects in the list are null, which is not handled or tested.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:163: Side effect of calling OwnerResource.findOwnerCompany() inside the loop/processing logic may have performance or caching implications not tested.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:199-200: No test verification that the new externalUsers and userUsers lists are correctly populated and serialized in the response DTO.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:141-144: New status "Waiting for External Feedback" added with specific color logic but lacks corresponding unit test coverage for the new branch.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:303: Default return value for slaColorRank changed from 3 to 4; verify this does not break sorting precedence for uncolored/null statuses in existing tests.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:315-316: New color code "#a855f7" handled in ranking logic requires explicit test case to ensure correct rank assignment (3) vs default (4).
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:138-144: Integration or end-to-end tests should be updated to validate the visual status mapping for the new "Waiting for External Feedback" state.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:1-124: No unit tests or integration tests provided for the new API resource class.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:38-39: Missing validation for ticketId range and role parameter format before database access.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:47-51: Race condition potential when creating external user without unique constraint enforcement.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:53-55: Logic flaw in else block assumes external user exists but doesn't handle non-external users correctly.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:99-103: resolveCompanyForRole uses hardcoded string comparison instead of enum or constant for roles.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:126: Missing unit tests for the requireRole method and its authorization logic (superuser vs user roles).
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:17: Missing unit tests for error handling paths such as missing ticket, missing company, or invalid email formats.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:35: Missing unit tests for the creation of new participant users and their association with the company.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:63: Missing unit tests for the removal of participants and validation of company ownership constraints.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:10: Missing unit tests for the add method's success path and correct redirect response generation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:170-179: Missing test coverage for externalUsers and userUsers query logic, specifically regarding null company handling and JPQL join correctness.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:170-179: Potential NPE risk in OwnerResource.findOwnerCompany() if the owner context is not properly initialized in test environments.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:188-190: Missing test coverage for normalizeDisplayStatus method, specifically for the "Waiting for External Feedback" edge case.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:203-205: Missing test coverage for the new constructor arguments externalUsers and userUsers in RoleTicketDetailResponse.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:229: Missing test coverage for normalizeDisplayStatus usage in SupportTicketSummary.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:101-104: Missing test coverage for new isExternal and isParticipant methods; these are logic branches that require unit tests to verify correct type checking and null safety.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:45: Missing test coverage for the early return path in getSessionUser when user is null; ensures the session token is correctly invalidated.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:1-151: No test coverage for the new ExternalUserPicker component; missing unit tests for render logic, state management, and API interactions.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:44-58: handleAdd lacks unit testing for API success/failure paths and UI state transitions (loading, toast notifications).
  • src/frontend/src/components/users/ExternalUserPicker.tsx:60-78: handleRemove lacks unit testing for API success/failure paths and UI state transitions.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:83-96: Select component rendering and filtering logic for available users is not covered by tests.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:118-133: Removal button rendering and interaction logic for existing users is not covered by tests.
  • src/frontend/src/utils/navigation.ts:35-39: Missing test coverage for the "superuser" and "tam" roles to verify the inclusion of the new "Users" navigation item.

Documentation

  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:95-96: Missing documentation for the new externalUsers field; purpose and usage context are unclear.
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:98-99: Missing documentation for the new userUsers field; naming is ambiguous and lacks explanation.
  • src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java:95-99: No Javadoc or inline comments explaining the distinction between tamUsers, externalUsers, and userUsers.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:33-34: Missing Javadoc or inline comments explaining the purpose and usage of the new TYPE_EXTERNAL and TYPE_PARTICIPANT constants.
  • src/backend/main/java/ai/mnemosyne_systems/model/User.java:36: Missing Javadoc or inline comments explaining the purpose of DISABLED_PASSWORD_HASH and why this specific value is used for disabled accounts.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:159-160: Missing Javadoc or inline comment explaining the security rationale for DISABLED_PASSWORD_HASH and the auto-generated username format.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:175-180: Missing documentation on the fallback logic for country/timezone selection in the bootstrap endpoint.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:227-235: Missing documentation for the requireRole method's role hierarchy and access control logic.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:237-250: Missing documentation for resolveCompanyForRole explaining the different scoping rules per role.
  • src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java:252-258: Missing documentation for the selectCountry fallback behavior.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:162-168: Missing JavaDoc or comments explaining the purpose of fetching externalUsers and userUsers or the distinction between them.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java:278-282: The normalizeDisplayStatus method lacks JavaDoc documentation describing its contract and the specific status normalization logic.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:141-144: Missing documentation or comment explaining the business logic or origin of the specific hex color code #a855f7 for the "Waiting for External Feedback" status.
  • src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java:315-317: Missing documentation or comment explaining why the new purple status is assigned rank 3 (higher priority than white, lower than red) in the color ranking system.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:34: Missing JavaDoc comment explaining the purpose, parameters, and return value of the add method.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:68: Missing JavaDoc comment explaining the purpose, parameters, and return value of the remove method.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:96: Missing JavaDoc comment for the private helper method requireRole.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:107: Missing JavaDoc comment for the private helper method resolveCompanyForRole.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java:75-76: The logic for checking if the external user belongs to the same company is inconsistent with the add method (line 60-62); add allows re-adding if already linked, while remove throws 404 if companies don't match, potentially causing confusion without comments explaining this asymmetry.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:30: Missing JavaDoc comment for the add method; the method modifies database state and handles complex business logic (user creation/lookup, company validation) which requires clear documentation of its contract and side effects.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:87: Missing JavaDoc comment for the remove method; similar to add, this method performs critical state changes and authorization checks that should be documented.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:106: Missing JavaDoc comment for the private requireRole helper method; the logic for distinguishing between "superuser" and "user" roles is specific and should be explained to prevent misuse or confusion.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:13: Import io.quarkus.elytron.security.common.BcryptUtil is unused; while not strictly a documentation issue, unused imports clutter the codebase and suggest incomplete cleanup or implementation.
  • src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java:64: The logic for linking a newly created participant user to the company (roleCompany.users.add(participantUser)) lacks a comment explaining why this association is necessary, especially since the subsequent check in the else block relies on company membership to validate existing users.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-179: Added logic to fetch external and user users lacks documentation explaining the business purpose of these specific user lists or why they are fetched via distinct queries here.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:167-179: The use of OwnerResource.findOwnerCompany() is a static call that introduces hidden coupling and potential side effects; this dependency should be documented or refactored to be explicit via injection.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:287-295: The normalizeDisplayStatus method hardcodes specific status strings ("Waiting for External Feedback") without documentation explaining why these specific mappings are required or if they are derived from configuration.
  • src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java:380-381: The new externalUsers and userUsers parameters in the constructor are added without JavaDoc comments defining their expected contents or nullability.
  • src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java:102-105: Missing Javadoc documentation for isExternal and isParticipant methods, which violates the existing code style observed in the file.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:33-42: Missing documentation for the ExternalUserPickerProps interface; props such as endpointSuffix and isClosed lack JSDoc descriptions explaining their purpose and defaults.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:44: The useJson hook usage for fetching external users lacks a comment explaining the data structure or error handling strategy, which is critical for this new component.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:52-53: handleAdd function lacks inline comments or JSDoc explaining the postForm payload structure and the specific API endpoint expectations.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:72-73: handleRemove function lacks documentation clarifying the removal logic and why removingId state is used for disabling the button.
  • src/frontend/src/components/users/ExternalUserPicker.tsx:99-106: The fallback logic for displaying user names (user.displayName || user.fullName || user.email) is undocumented; it is unclear if this order is intentional or if fullName is deprecated.
  • src/frontend/src/types/domain/tickets.ts:48: Missing JSDoc comment for the new externalUsers property, leaving its purpose and usage undocumented.
  • src/frontend/src/types/domain/tickets.ts:49: The property name userUsers appears to be a typo or unclear naming convention and lacks documentation to clarify its intent.

Conclusion

orangu rejects this patch

  • Rejected: src/backend/main/java/ai/mnemosyne_systems/model/Ticket.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/model/User.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/ExternalUserApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/SuperuserTicketApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/SupportTicketViewSupport.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/TicketExternalUsersApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/TicketParticipantUsersApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/resource/UserTicketApiResource.java
  • Rejected: src/backend/main/java/ai/mnemosyne_systems/util/AuthHelper.java
  • Rejected: src/frontend/src/components/users/ExternalUserPicker.tsx
  • Rejected: src/frontend/src/pages/SupportTicketDetailPage.tsx
  • Rejected: src/frontend/src/types/domain/tickets.ts
  • Rejected: src/frontend/src/utils/navigation.ts

Generated by: orangu 0.7.0 (bartowski/Qwen_Qwen3.6-35B-A3B-GGUF)

@jesperpedersen

Copy link
Copy Markdown
Contributor

@jesperpedersen

Copy link
Copy Markdown
Contributor

@trxvorr Can you put a half an eye on this ? The security part

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 665e3cd to 460ea6b Compare June 14, 2026 18:27
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 We still need the "External" menu for all roles except admin. At least I don't see it for user

@jesperpedersen

Copy link
Copy Markdown
Contributor

@trxvorr Take what is correct from the auto review and create new issues for them

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 f.ex. for superuser we can fold it into the Users menu and have a drop-down of the type (User or External)

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch 2 times, most recently from bc0273e to b2a97fd Compare June 16, 2026 18:39
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 Just call the menu "External" for User. http://localhost:8080/user/externals/new doesn't need the "Delete user" button (and it is in the wrong position - Delete is always lower-left)

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 "Leave blank..." doesn't make sense

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 "New External Contributor" -> "External Contributor"

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 "Password" takes both panels - there is no "Verify password"

@jesperpedersen

jesperpedersen commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@sksingh2005 We don't need the drop-down for "Users" we will filter on the page itself - or just show their role

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 Lets start with these high level feedback - make sure that the pages look at the existing pages

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch 2 times, most recently from 2ab7b4f to a34c4bf Compare June 17, 2026 13:55
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 You have to go through all roles - they can all create External users. User, Superuser and TAM for their assigned company. Support for all companies

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from a34c4bf to 11dc818 Compare June 17, 2026 19:28
@sksingh2005

Copy link
Copy Markdown
Collaborator Author

@sksingh2005 You have to go through all roles - they can all create External users. User, Superuser and TAM for their assigned company. Support for all companies

yes was working on that part only.

@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 Login as Support -> Users -> Create - there is no External option.

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 11dc818 to 0232c56 Compare June 18, 2026 07:47
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 TAM...

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 0232c56 to 2bf0047 Compare June 18, 2026 17:44
@jesperpedersen

Copy link
Copy Markdown
Contributor

@sksingh2005 TAM should have the "External" menu - like User

@sksingh2005 sksingh2005 force-pushed the feature/external-contributors branch from 2bf0047 to 883804e Compare June 19, 2026 14:23
@jesperpedersen

jesperpedersen commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Superuser has "Users" and "External" in menu bar... It should be in the a drop-down - which it has

@jesperpedersen

Copy link
Copy Markdown
Contributor

User: Where is the password input ?

@jesperpedersen

Copy link
Copy Markdown
Contributor

TAM: Has "Users" and "External" in the menu bar as well

@jesperpedersen

Copy link
Copy Markdown
Contributor

Support: Same...

@jesperpedersen

Copy link
Copy Markdown
Contributor

If there is a "Users" in the menu bar then there isn't an "External"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants