Skip to content

fix(predicate): adds exclusive method to get user notification data#498

Merged
pedroanastacio merged 4 commits intostagingfrom
p2/fix/prevents-exposure-of-user-email
Mar 13, 2026
Merged

fix(predicate): adds exclusive method to get user notification data#498
pedroanastacio merged 4 commits intostagingfrom
p2/fix/prevents-exposure-of-user-email

Conversation

@pedroanastacio
Copy link
Contributor

@pedroanastacio pedroanastacio commented Mar 6, 2026

Description

The endpoint GET - /predicate/:predicateId was returning more information than it should have, in this case the user's email address. To obtain notification information about a user, an internal method must be created in the API so as not to expose critical information.}

Summary

  • Adds getNotificationInfo method to obtain user notification information.
  • Moves email sending logic to Notifications service.
  • Removes email and notify from the return of the endpoint GET - /predicate/:predicateId.

Checklist

  • I reviewed my PR code before submitting
  • I ensured that the implementation is working correctly and did not impact other parts of the app
  • I mentioned the PR link in the task

@pedroanastacio pedroanastacio requested a review from guimroque March 6, 2026 14:23
Copy link
Member

@guimroque guimroque left a comment

Choose a reason for hiding this comment

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

Code Review

Summary

This PR successfully addresses a security vulnerability by preventing exposure of user email addresses in the predicate endpoint. The refactoring moves notification logic to a dedicated service and introduces a new method to safely retrieve user notification data.

Strengths

  • Excellent security fix removing email exposure from public endpoint
  • Clean separation of concerns moving notification logic to NotificationService
  • Proper error handling with structured logging
  • Good use of parallel processing for notifications and emails

Issues

  • 1 critical, 1 important, 1 suggestion

@pedroanastacio pedroanastacio merged commit cc61b6b into staging Mar 13, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants