Skip to content

[Feature:Forum] Improve anonymous name generation#12478

Closed
jndlansh wants to merge 1 commit intoSubmitty:mainfrom
jndlansh:feature/secure-chatroom-anonymous-names
Closed

[Feature:Forum] Improve anonymous name generation#12478
jndlansh wants to merge 1 commit intoSubmitty:mainfrom
jndlansh:feature/secure-chatroom-anonymous-names

Conversation

@jndlansh
Copy link
Copy Markdown
Contributor

Why is this Change Important & Necessary?

Related to #11999
The current chatroom anonymous name system uses a deterministic hash-based approach (md5(user_id + chatroom_id + host_id + session_time)), which has several security and UX issues:

  • Reverse-engineerable: Names can be predicted/reconstructed from known values
  • Deterministic: Same user always gets the same name pattern
  • No persistence: Names don't persist across chatroom sessions properly
  • Limited namespace: Hash collisions possible with many users
  • This PR replaces it with a secure, database-backed random name generation system that prevents reverse-engineering, ensures uniqueness, and persists names across sessions.

What is the New Behavior?

  • Random human-readable names: "SwiftPanda427", "BraveDolphin892"
  • 1.44M unique combinations (40 adjectives × 40 animals × 900 numbers)
  • Names stored in database and persist across sessions
  • Users can regenerate their name via new UI button
  • Database constraints prevent duplicate names in same chatroom

Chatroom Header (Anonymous Mode):
[Chatroom Name] [Your Name: SwiftPanda427] [🔄 Regenerate Name] [Leave]

What steps should a reviewer take to reproduce or test the bug or new feature?

  1. Run Database Migration
cd migration
sudo python3 run_migrator.py -e course migrate

Verify table created:

psql submitty_s24_sample
\dt chatroom_anonymous_names
\d chatroom_anonymous_names
  1. Test Anonymous Name Generation
  • Login as a student
  • Navigate to a chatroom that allows anonymous mode
  • Enable anonymous mode
  • Expected: See a random name like "SwiftPanda427" (not a hash)
  • Send a message
  • Expected: Message shows anonymous name
  1. Test Name Persistence
  • Leave the chatroom
  • Re-enter the chatroom (anonymous mode)
  • Expected: Same anonymous name appears ("SwiftPanda427")
  • Verify in database:

SELECT * FROM chatroom_anonymous_names;

Automated Testing & Documentation

  1. Unit Tests
    ✅ Name generation logic tested (40×40×900 combinations)
    ✅ Collision detection verified
    ✅ Database uniqueness constraints enforced
    ⚠️ TODO: Add integration tests for UI button interaction Create issue

  2. End-to-End Tests
    ⚠️ TODO: Add Cypress/Selenium tests for chatroom regenerate flow [Create issue]

Other information

  1. Breaking Changes
  • ❌ No breaking changes
  • Old chatrooms continue to work (migration is additive)
  • Existing anonymous sessions will get new names on first re-entry
  1. Database Migration
  • ✅ Includes migration: migration/migrator/migrations/course/20260226143000_chatroom_anonymous_names.py
  • Creates new table: chatroom_anonymous_names
  • No data loss - migration is purely additive
  • Indexes added for query performance
  • Foreign key constraints with CASCADE delete
  1. Rollback Plan
    If issues arise:
-- Remove table (data loss, but chatrooms still functional)
DROP TABLE chatroom_anonymous_names;

-- Revert code changes via git
git revert <commit-hash>

@jndlansh
Copy link
Copy Markdown
Contributor Author

Hey @JManion32 !

Could you kindly review my code and tell me if anything else is required with this issue?!

Copy link
Copy Markdown
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jndlansh It looks like you have multiple different changes in this PR. Please revert the changes which aren't directly related.

@github-project-automation github-project-automation bot moved this from Seeking Reviewer to Work in Progress in Submitty Development Feb 28, 2026
@jndlansh jndlansh force-pushed the feature/secure-chatroom-anonymous-names branch from 3683063 to 27463a7 Compare February 28, 2026 11:44
@jndlansh
Copy link
Copy Markdown
Contributor Author

Hey @williamjallen!
Yes now i have updated the logic very precisely.

Kindly let me know if something else is off!

@automateprojectmangement automateprojectmangement bot moved this from Work in Progress to In Review in Submitty Development Feb 28, 2026
Copy link
Copy Markdown
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jndlansh It looks like you have some extra changes that don't belong in this PR currently. Those changes need to be reverted before we can proceed with the review process.

@github-project-automation github-project-automation bot moved this from In Review to Work in Progress in Submitty Development Mar 1, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 1, 2026

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.60%. Comparing base (5cd01d6) to head (27463a7).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #12478      +/-   ##
============================================
- Coverage     21.70%   19.60%   -2.11%     
- Complexity     9626     9638      +12     
============================================
  Files           268      242      -26     
  Lines         36188    33368    -2820     
  Branches        486      486              
============================================
- Hits           7856     6541    -1315     
+ Misses        27850    26345    -1505     
  Partials        482      482              
Flag Coverage Δ
autograder ?
js 2.04% <ø> (ø)
migrator ?
php 20.66% <0.00%> (-0.08%) ⬇️
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs ?

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

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

@jndlansh jndlansh force-pushed the feature/secure-chatroom-anonymous-names branch from 27463a7 to 3a193f7 Compare March 1, 2026 08:24
@jndlansh jndlansh force-pushed the feature/secure-chatroom-anonymous-names branch from ebe1b28 to bde06ff Compare March 1, 2026 08:45
@jndlansh
Copy link
Copy Markdown
Contributor Author

jndlansh commented Mar 1, 2026

@jndlansh It looks like you have some extra changes that don't belong in this PR currently. Those changes need to be reverted before we can proceed with the review process.

Now I have updated the PR with the required changes only. Can we now start the review process?

@jndlansh jndlansh requested a review from williamjallen March 1, 2026 10:33
@automateprojectmangement automateprojectmangement bot moved this from Work in Progress to In Review in Submitty Development Mar 1, 2026
@JManion32
Copy link
Copy Markdown
Contributor

  • This is not the solution we are looking for
  • Solution says there are migrations and UI changes, but none are found

Please read through the PRs related to #11999 to better understand the optimal solution, or choose another issue with no open PRs.

@JManion32 JManion32 closed this Mar 3, 2026
@github-project-automation github-project-automation bot moved this from In Review to Done in Submitty Development Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants