chat interface and improve authentication flow#50
Merged
Conversation
- Updated index.html to include new CSS files for authentication, chat, and verification styles. - Removed the compose button from the navbar and adjusted the inbox view to reflect new chat terminology. - Refactored app.js to remove the compose view and streamline navigation. - Enhanced the inbox view in views.js to support conversation grouping and improved message rendering. - Updated message retrieval logic in routes.py to handle both sent and received messages in a single query. - Introduced new CSS styles for authentication, chat, and verification views to improve UI consistency and aesthetics.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR reorganizes the client-web frontend from a monolithic views module into separate auth, inbox, and helper modules while persisting crypto key material to sessionStorage for page-reload resilience. The server extends JWT lifetime to 1 hour, enforces password complexity at registration, and refactors message endpoints with UNION ALL queries, revised authorization semantics, and new sender metadata for TOFU verification. ChangesWeb Client UI Refactoring and Session Management
Server Authentication and Session Updates
Server Message Endpoints Refactored
Documentation and Project Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
server/app/__init__.py (1)
60-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the comment to reflect the new token expiry.
The comment still references "15 min" but tokens now expire after 1 hour (line 45).
📝 Proposed fix
- # In-memory JTI blocklist — tokens expire in 15 min so the set stays small. + # In-memory JTI blocklist — tokens expire in 1 hour so the set stays small. # Cleared on server restart, which is acceptable: a restarted server issues new tokens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/app/__init__.py` at line 60, Update the inline comment that currently reads "# In-memory JTI blocklist — tokens expire in 15 min so the set stays small." to reflect the new token expiry of one hour (e.g., "tokens expire in 1 hour"), ensuring the JTI blocklist comment accurately describes current behavior near the in-memory JTI/blocklist declaration in server.app.__init__.py.server/app/auth/routes.py (1)
233-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce password complexity for new passwords.
The
/change-passwordendpoint does not validatenew_passwordagainst the same complexity rules enforced at registration (lines 50-63). Users can change to weak passwords, defeating the security requirement.🔒 Proposed fix: extract validation and apply to both endpoints
Add a helper function before the
registerroute:def _validate_password_complexity(password): """Validate password meets complexity requirements. Returns: list: Error messages for failed requirements, empty if valid. """ errors = [] if len(password) < 8: errors.append('at least 8 characters') if not re.search(r'[A-Z]', password): errors.append('one uppercase letter') if not re.search(r'[a-z]', password): errors.append('one lowercase letter') if not re.search(r'[0-9]', password): errors.append('one number') if not re.search(r'[^A-Za-z0-9]', password): errors.append('one special character') return errorsThen update
/register(lines 50-63):pw = data['password'] - pw_errors = [] - if len(pw) < 8: - pw_errors.append('at least 8 characters') - if not re.search(r'[A-Z]', pw): - pw_errors.append('one uppercase letter') - if not re.search(r'[a-z]', pw): - pw_errors.append('one lowercase letter') - if not re.search(r'[0-9]', pw): - pw_errors.append('one number') - if not re.search(r'[^A-Za-z0-9]', pw): - pw_errors.append('one special character') + pw_errors = _validate_password_complexity(pw) if pw_errors: return jsonify({'error': f"Password must contain: {', '.join(pw_errors)}"}), 400And add validation to
/change-passwordbefore line 233:+ pw_errors = _validate_password_complexity(data['new_password']) + if pw_errors: + return jsonify({'error': f"New password must contain: {', '.join(pw_errors)}"}), 400 + new_hash = ph.hash(data['new_password'])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/app/auth/routes.py` at line 233, Extract the password complexity checks into a helper function named _validate_password_complexity(password) that returns a list of error messages (empty if valid) and include checks for min length, uppercase, lowercase, digit, and special char; call this helper from the register route (replacing the inline checks) and from the change-password flow before hashing (i.e., before new_hash = ph.hash(data['new_password'])), and if the helper returns errors return the same error response used in register (400 with descriptive message) so change-password enforces the same complexity rules.client-web/js/api.js (1)
157-171:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear cached sent plaintext on logout.
renderInbox()writes sent bodies tosessionStorageassent_plain_*, but logout only removes auth/key fields. That leaves previous-session plaintext readable after sign-out in the same tab.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client-web/js/api.js` around lines 157 - 171, The logout() function in client-web/js/api.js fails to clear sent-message plaintext that renderInbox() stores under sessionStorage keys like "sent_plain_*", leaving previous-session plaintext accessible after sign-out; update logout() to iterate sessionStorage keys and remove any entries whose key startsWith "sent_plain_" (and any other "sent_*" keys you deem necessary) after clearing tokens/keys so all cached sent plaintext is removed on logout. Ensure the removal runs in the same function (logout) and does not rely on other UI flows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client-web/js/api.js`:
- Around line 43-44: The code drops _sessionUserId across reload/unlock causing
getUserId() to return null and breaking renderInbox() send/forward paths;
persist the user_id into sessionStorage when setting it and restore it when the
app initializes/unlocks. Update the functions related to _sessionUserId (e.g.,
getUserId, clearUserId and whatever setter around lines 118-119) to read/write
sessionStorage["user_id"] (or a consistent key) so that on reload/unlock the
code restores _sessionUserId from sessionStorage before renderInbox() runs and
before any send/forward handlers call getUserId().
In `@client-web/js/app.js`:
- Around line 40-45: isAuthView currently includes 'verify' causing the
auth-mode layout to apply to the public verification page; update the logic that
defines isAuthView (the expression that sets const isAuthView) so it excludes
'verify' (e.g., only treat actual auth routes like 'login' or 'signup' as auth
views), leaving the navbar.hidden and appEl.classList.toggle('auth-mode',
isAuthView) usage unchanged so the verify view renders with the normal
full-width layout.
In `@client-web/js/views/inbox.js`:
- Around line 600-647: The forward case currently re-encrypts and calls
api.forwardMessage directly, which bypasses sendFromThread() (and its TOFU
confirmation) and the sent_plain_* recovery path; change the flow to obtain
plaintext the same way (using decryptMessage as you already do) but then call
sendFromThread() (or a new helper that wraps sendFromThread) to perform the
actual send/forward so TOFU checks and sent_plain_* bookkeeping are preserved;
update the forward branch to pass the decrypted plaintext, recipientUsername
(and original message id/context if needed) into sendFromThread()/the wrapper
and remove the direct call to api.forwardMessage/encryptMessage so forwarded
messages follow the same send path and recovery semantics.
- Around line 414-458: The poll handler (pollInterval) currently only appends
unseen IDs and never reconciles deletes/revokes in currentConvMap or knownIds;
update the polling logic after receiving data.messages to detect messages marked
deleted/revoked (e.g., a flag like msg.deleted or msg.revoked) and for each such
message remove or update that message in currentConvMap and knownIds, then
remove any corresponding DOM bubble and call renderConvList(currentConvMap,
activePartner) to refresh sidebar previews and reopened threads; also ensure the
same reconciliation is applied in the other polling area around lines 571-579
and when tryDecrypt returns null/placeholder so revoked content doesn't get
reinserted via buildBubble/handleAction.
- Around line 345-358: The async getUser(...) callback can write a stale
fingerprint if the user switches threads; to fix, capture the expected partner
id before awaiting (e.g., const expectedPartner = partner) and, right before any
DOM updates (before fpEl.textContent / fpEl.style changes), verify the
currently-open thread still matches expectedPartner (for example compare a
stable source-of-truth such as a currentPartner variable or an attribute on the
thread-fingerprint element like fpEl.dataset.partner or another thread ID in the
UI); if they differ, return early and do not update the element. Keep existing
logic using getUser, keyFingerprint and tofuCheck but gate all DOM writes with
that equality check to prevent overwriting with stale data.
- Around line 181-191: Cache sender keys by the actual key value instead of only
msg.sender_username: compute a stable key identifier from
msg.sender_x25519_public_key (e.g. the base64 string itself), then look up
senderKeyCache using that identifier; if not present, call
tofuCheck(msg.sender_username, msg.sender_x25519_public_key) and import the key
with crypto.subtle.importKey, storing it under the key identifier. Additionally,
detect when an incoming msg.sender_x25519_public_key differs from any previously
cached key for msg.sender_username and run tofuCheck to surface key-rotation
warnings, then update the cache to the new key identifier so decryption uses the
correct imported key (references: senderKeyCache, tofuCheck,
msg.sender_x25519_public_key, msg.sender_username, crypto.subtle.importKey).
In `@server/app/messages/routes.py`:
- Around line 242-244: The revoke-forwarding check only blocks forwarding back
to the original recipient (uses message['is_revoked'] and recipient['id'] ==
message['recipient_id']) but doesn’t prevent other forwards; update the logic
based on the intended policy: either block all forwards of revoked messages by
changing the check to simply if message['is_revoked']: return 403, or if you
only want to prevent the original recipient from further distributing it, change
the check to compare the acting user (e.g. current_user['id'] or sender_id used
in this route) against message['recipient_id'] (i.e. if message['is_revoked']
and actor_id == message['recipient_id']: return 403), and add a clarifying
comment describing the chosen design.
---
Outside diff comments:
In `@client-web/js/api.js`:
- Around line 157-171: The logout() function in client-web/js/api.js fails to
clear sent-message plaintext that renderInbox() stores under sessionStorage keys
like "sent_plain_*", leaving previous-session plaintext accessible after
sign-out; update logout() to iterate sessionStorage keys and remove any entries
whose key startsWith "sent_plain_" (and any other "sent_*" keys you deem
necessary) after clearing tokens/keys so all cached sent plaintext is removed on
logout. Ensure the removal runs in the same function (logout) and does not rely
on other UI flows.
In `@server/app/__init__.py`:
- Line 60: Update the inline comment that currently reads "# In-memory JTI
blocklist — tokens expire in 15 min so the set stays small." to reflect the new
token expiry of one hour (e.g., "tokens expire in 1 hour"), ensuring the JTI
blocklist comment accurately describes current behavior near the in-memory
JTI/blocklist declaration in server.app.__init__.py.
In `@server/app/auth/routes.py`:
- Line 233: Extract the password complexity checks into a helper function named
_validate_password_complexity(password) that returns a list of error messages
(empty if valid) and include checks for min length, uppercase, lowercase, digit,
and special char; call this helper from the register route (replacing the inline
checks) and from the change-password flow before hashing (i.e., before new_hash
= ph.hash(data['new_password'])), and if the helper returns errors return the
same error response used in register (400 with descriptive message) so
change-password enforces the same complexity rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d4269d69-15e1-4c8f-830d-21ecef6550be
⛔ Files ignored due to path filters (1)
client-web/images/bows_background.pngis excluded by!**/*.png
📒 Files selected for processing (17)
.gitignoreclient-cpp/README.mdclient-web/README.mdclient-web/css/auth.cssclient-web/css/base.cssclient-web/css/chat.cssclient-web/css/verify.cssclient-web/index.htmlclient-web/js/api.jsclient-web/js/app.jsclient-web/js/views.jsclient-web/js/views/auth.jsclient-web/js/views/helpers.jsclient-web/js/views/inbox.jsserver/app/__init__.pyserver/app/auth/routes.pyserver/app/messages/routes.py
💤 Files with no reviewable changes (1)
- client-web/js/views.js
sarahmc253
approved these changes
Jun 2, 2026
This was referenced Jun 2, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Revamp the chat interface to resemble popular messaging apps, streamline navigation, and enhance user experience. Implement password validation, TOFU key pinning, and improve message handling. Extend JWT expiry for better session management and introduce automatic message polling to keep the inbox updated. Adjust frontend styles for consistency and usability.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores