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

feat: do not allow non-members to modify member list #6436

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Jan 14, 2025

This issue was ignored for the initial implementation of #6401 but it seems it does not hurt to ignore membership changes from non-members. It's still better to create a new group if you want someone removed to be unable to get back into the group or write into it, but this change also provides sufficient protection most of the time without breaking existing tests.

@link2xt
Copy link
Collaborator Author

link2xt commented Jan 14, 2025

Best reviewed with "hide whitespace" on.

@link2xt link2xt requested review from iequidoo and Hocuri January 14, 2025 01:29
@iequidoo
Copy link
Collaborator

iequidoo commented Jan 14, 2025

In #6401 you wrote:

Any attempts to e.g. ignore adding self to the group will result in "member is not part of the group" errors due to reordering in cases without any attackers.

Is there any plan of fixing this? Maybe add such messages as "partially downloaded" (but with some explanation)? This way they are hidden because may be malicious and also when the user downloads such a message later and the sender is already a member, it should be shown correctly. The downside is that the message is downloaded twice, but that shouldn't happen frequently.

@link2xt
Copy link
Collaborator Author

link2xt commented Jan 14, 2025

Need to test what actually happens when someone tries to add self to verified group before this PR. They should not be able to "introduce self". If they can, that's a bug, and if they cannot, then there is an error in case of reordering anyway.

EDIT: we already have test_verified_member_added_reordering test.

@link2xt link2xt force-pushed the link2xt/non-member-member-list branch from f7619b9 to 3e7b662 Compare January 15, 2025 16:43
@link2xt link2xt merged commit 3e7b662 into main Jan 15, 2025
24 checks passed
@link2xt link2xt deleted the link2xt/non-member-member-list branch January 15, 2025 16:44
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.

2 participants