Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hide metadata of private notes #8641

Closed
wants to merge 18 commits into from

Conversation

Johann150
Copy link
Contributor

@Johann150 Johann150 commented May 10, 2022

What

  • the getNote getter is expanded to also check visibility as this is a common task across all API endpoints
  • some uses of Notes.find or similar are replaced by uses of getNote so the visibility is checked properly
  • Notes.pack will return an error if the note is not visible to the specified user instead of only hiding metadata
  • Notes.packMany is modified to not throw an error, but only filters out notes that would have raised an error
  • all uses of Notes.pack properly handle the new error
  • adjust tests

Why

fix #8317

Additional Info

The hideNote function has been removed. This has the additional advantage that it removes duplicated code for checking the visibility of a note already in isVisibleForMe.

Users will no longer be able to see notes that are not visible to them. The previous behaviour was to show the note just with the text replaced by a message that the note is not visible to the user. The new behaviour is that the server claims that the note does not exist at all if it is not visible to the user.

Users will no longer be able to reply to or renote notes that are not visible to them.

@github-actions github-actions bot added packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR labels May 24, 2022
@Johann150 Johann150 marked this pull request as ready for review June 1, 2022 08:51
@Johann150 Johann150 force-pushed the hidden-notes branch 2 times, most recently from 533a765 to 7a76b5b Compare June 9, 2022 06:49
@Johann150 Johann150 marked this pull request as draft June 9, 2022 10:18
@Johann150
Copy link
Contributor Author

I discovered some issues with this, it needs more testing.

@Johann150 Johann150 marked this pull request as ready for review June 14, 2022 15:06
@Johann150
Copy link
Contributor Author

Rebased to remove the changes that were already merged in #8838.

Johann150 added 17 commits June 29, 2022 09:17
Raise an error When a note is not visible to the requesting user.
If a note is not visible to the requesting user, an error will be raised.
Instead of just hiding specific fields, the entire note is hidden. This means
that metadata of the note such as who is the author, when was it sent are
completely hidden.
Instead of packing the note for public user before passing it to
streams, the note is now either packed for the user the respective
stream belongs to (`mainStream`) or not packed at all and then packed
later (`notesStream`).

Because this is a new common task between different channels, a shared
implementation of packing a note from notesStream is created. This
implementation will simply skip a note if it is not visible to the user
that the channel belongs to.
The `isHidden` attribute is not being set any more and is thus removed.
Handling in the client is no longer necessary.
Packing with detail can cause an error if the reply or renote
are not visible to the user, even though the original note is
visible to the user.
@Johann150
Copy link
Contributor Author

There are many merge conflicts and I do not have time or motivation to refactor to the new code layout.

@Johann150 Johann150 closed this Dec 4, 2022
@Johann150 Johann150 deleted the hidden-notes branch December 4, 2022 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Messages that aren't supposed to be accesible are still (kind of) visible
1 participant