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

More informative chats' name and title for WhatsApp and Telegram (UFED parser) #2284

Conversation

marcosammoura
Copy link
Contributor

Currently, chats' names lack information concerning the user account they are related to.
Although this is a broad scenario concerning chat apps, the proposed solution targets are WhatsApp and Telegram chats processed using UFED parser.

I propose that chat's names include account info, which provides at least two benefits:
1 - it becomes easy to identify the account to which each chat is related.
2 - it allows for chats' grouping by account (via chat name sorting).
I do also propose that chat's names include its type (e.g., private, group, status, broadcast), which makes it easier to spot chats and to eventually draw conclusions about communications much faster

In addition, the proposed solution also provides a more informative title to the aforementioned chats using UFED parser, which currently is the chat id. The new title aligns with the proposed chats' naming scheme, highlighting the chat type and the corresponding identification (e.g., participant name, group name, brodcast name).

@lfcnassif
Copy link
Member

Thank you @marcosammoura! Improving names and titles of chats handled by UFEDChatParser is an old idea!

Wouldn't it be possible to remove the WhatsApp/Telegram restriction, applying the same rules when possible? I believe checking if each used metadata value is not null neither empty is enough.

@marcosammoura
Copy link
Contributor Author

@lfcnassif, thank you for the feedback. I think your approach is a much better and generic solution. I will change the code.

@aberenguel
Copy link
Contributor

Great @marcosammoura!
Can the restriction to WhatsApp and Telegram in reportGenerator be removed?

@marcosammoura
Copy link
Contributor Author

marcosammoura commented Aug 13, 2024

@lfcnassif and @aberenguel, I have improved the code. Now it's not only generic, but it also deals with WhatsApp and Telegram specifics. Regarding these two apps, different chat types are stored in UFED XML report according to the type of communication. On the other hand, as far as I could test, this isn't the case for Facebook, Facebook Messenger, and Intagram, for instance - concerning these apps, "Unknown" chat type is stored in UFED XML report, irrespective to type the communication.
I'm testing the new code with some cases, and once I finish I will push a new commit.

@marcosammoura
Copy link
Contributor Author

marcosammoura commented Aug 14, 2024

@lfcnassif and @aberenguel, the code was tested and now is ready and available in the latest commit.

@lfcnassif
Copy link
Member

Thank you very much @marcosammoura for this improvement! Unfortunately I'm not sure when I'll have time to review this, but I'll try to do as soon as possible.

@wladimirleite wladimirleite self-requested a review October 17, 2024 19:47
@wladimirleite
Copy link
Member

@lfcnassif and @marcosammoura, I am starting to review this, ok?

@wladimirleite
Copy link
Member

wladimirleite commented Oct 17, 2024

Two initial observations:

  1. There is a large intersection with code changes from Chat minor improvements #2286. As this is a much smaller PR, and we are not sure if there will be available time to review the other PR before 4.2 release, I think it is ok to review and include changes from this PR. Later, we may decide to use all of it, or just part, merging with the code and ideas implemented by the other PR. @lfcnassif, do you agree with this approach?
  2. The goal of this PR seems a great idea, and having more informative names will be helpful, however my first impression is that names after the proposed changes became a bit too long. I am running a test on a few UFDR's from real cases, and will propose some changes, to reduce the new names a bit. I will post some samples, so we can decide between more informative and more concise/readable names.

@lfcnassif
Copy link
Member

  1. I think it is ok to review and include changes from this PR. Later, we may decide to use all of it, or just part, merging with the code and ideas implemented by the other PR. @lfcnassif, do you agree with this approach?

Sure, I agree! But keeping the new behavior/chat names later is important, regardless if we keep all code changes or part of them when resolving conflicts with @aberenguel's PR.

@marcosammoura
Copy link
Contributor Author

Thanks for your observations, @wladimirleite,

Names got really very long, so it would be great to use more concise/readable names.

I am completely with you @lfcnassif that the most important now is to make sure @aberenguel's PR prevails, since it is a much broader solution that also tackles this issue.

@lfcnassif
Copy link
Member

Hi @marcosammoura, actually I meant we ideally should keep the final behavior of this PR later after merging it, even if it means changing a bit @aberenguel' PR, to avoid changing behavior every release.

@wladimirleite
Copy link
Member

While reviewing the changes, I am taking a look at @aberenguel's PR (the part that intersects with this PR). if I found something missing here and present there, regarding chat names, I will try to add to this PR, to avoid future changes.

@wladimirleite
Copy link
Member

About the long names, item names after this PR changes look like this:

[email protected]_PhoneOwner_Dr.Gurgel([email protected])_OneOnOne_Willian Smith Senior([email protected])
[email protected]_PhoneOwner_Dr.Gurgel([email protected])_Group_Gurgel Family

Although it is informative, it seems a bit too long (at least for me).
One problem that I see, it will help for sorting, but in most cases, when there is only one account, the variable part will start after the "OneOnOne". In the example above, there will be 111 characters in the fixed part. Also, usually there will be some redundancy (in this example, taken from a real case, Account and Phone Owner are similar).

My suggestions:

  • If the account is available, use it. Otherwise, try to use the phone owner.
  • Do not add the fixed words "Account" or "Phone Owner".
  • For the phone owner and the other party account, use only the name if available, otherwise use the number (without the "@s.whatsapp.net" suffix).
  • Omit "OneOnOne". Use chat type qualifiers only for group, broadcast or channel.
Chat_WhatsApp_5511922222222_Willian Smith Senior
Chat_WhatsApp_5511922222222_Group_Gurgel Family

@wladimirleite
Copy link
Member

It is important to notice that #2286 will add/organize some metadata related to chats, so it will also be possible to apply filters or sort using these properties.

@marcosammoura
Copy link
Contributor Author

It is important to notice that #2286 will add/organize some metadata related to chats, so it will also be possible to apply filters or sort using these properties.

@wladimirleite, with metadata and filters this solution will be great.

Copy link
Member

@wladimirleite wladimirleite left a comment

Choose a reason for hiding this comment

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

@marcosammoura, I would appreciate it if you can make a quick test with the changes I made when you get some time.
Thank you once again for this PR!

@lfcnassif, as I mentioned before, I reduced the items' names, so they should be informative but not excessively long. This balance is subjective, so we could add back some of the information I removed, if necessary.
If we agree that names are fine, it should be ready for merging.

I included 3 additional minor related changes:

  • Use localized string for the chat types used in item's name and preview title (Group, Status, Broadcast and Unknown);
  • Use different backgrounds/top bar color for WhatsApp, Telegram and others (a new "generic" grayish background). Some users (including myself) found it a bit confusing to use WhatsApp background for other sources.
  • "Favicon" (displayed only when the HTML is opened externally) wasn't working. I fixed it, and set different icons for WhatsApp, Telegram and other sources.

@marcosammoura
Copy link
Contributor Author

@wladimirleite, this set of improvements made things much better. Great job.

@lfcnassif
Copy link
Member

Thank you @marcosammoura and @wladimirleite for your work on this!

@lfcnassif lfcnassif merged commit b3d8fdd into sepinf-inc:master Oct 19, 2024
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.

4 participants