fix(chat): derive private chat sender from authenticated security con… - #317
Open
hazelr125 wants to merge 4 commits into
Open
fix(chat): derive private chat sender from authenticated security con…#317hazelr125 wants to merge 4 commits into
hazelr125 wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request addresses issue #314 by preventing private chat sender impersonation: the backend now derives the sender from the authenticated Spring Security context, and the frontend stops sending sender in the query string.
Changes:
- Backend: Update
GET /api/private-chats/betweento take onlyreceiverfrom the request and derivesenderfromAuthentication#getName(). - Frontend: Update the private chat API client and calling code to omit the
senderquery parameter. - Frontend: URL-encode the
receiverusername when calling the endpoint.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/app/services/private-chat.service.ts | Removes sender from the request and encodes receiver in the URL. |
| frontend/src/app/pages/home/home.component.ts | Updates the call site to match the new service signature. |
| backend/src/main/java/vaultWeb/controllers/PrivateChatController.java | Derives sender from Spring Security Authentication and only accepts receiver as a request parameter. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
59
to
65
| public PrivateChatDto getOrCreatePrivateChat( | ||
| @RequestParam String sender, @RequestParam String receiver) { | ||
| @RequestParam String receiver, Authentication authentication) { | ||
| if (authentication == null) { | ||
| throw new UnauthorizedException("User not authenticated"); | ||
| } | ||
| String sender = authentication.getName(); | ||
| PrivateChat chat = privateChatService.getOrCreatePrivateChat(sender, receiver); |
Comment on lines
20
to
25
| getOrCreatePrivateChat( | ||
| username1: string, | ||
| username2: string, | ||
| ): Observable<PrivateChatDto> { | ||
| return this.http.get<PrivateChatDto>( | ||
| `${this.apiUrl}/private-chats/between?sender=${username1}&receiver=${username2}`, | ||
| `${this.apiUrl}/private-chats/between?receiver=${encodeURIComponent(username2)}`, | ||
| ); |
Comment on lines
59
to
65
| public PrivateChatDto getOrCreatePrivateChat( | ||
| @RequestParam String sender, @RequestParam String receiver) { | ||
| @RequestParam String receiver, Authentication authentication) { | ||
| if (authentication == null) { | ||
| throw new UnauthorizedException("User not authenticated"); | ||
| } | ||
| String sender = authentication.getName(); | ||
| PrivateChat chat = privateChatService.getOrCreatePrivateChat(sender, receiver); |
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
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.
Summary
Fixed the private chat impersonation issue by deriving the authenticated sender from the Spring Security context instead of trusting a request parameter. The endpoint now uses the current user from Authentication#getName() and only accepts the receiver from the request.
Linked issue
Closes #314
How to test
a. Authenticate as a user and call /api/private-chats/between?receiver=otherUser
b. Confirm the chat is created/retrieved for the authenticated user only
c. Try passing a different sender value in the request and verify it is ignored
d. Confirm the endpoint still works normally for valid authenticated requests
Notes / Risk
a. Low risk: this is a controller-level authorization fix
b. Frontend callers should stop sending sender in the query string
c. No database migration or config change required