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: save used invite #330

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mafineeek
Copy link

Please make sure to check the following tasks before opening and submitting a PR

  • I understand and have followed the contribution guide
  • I have tested my changes locally and they are working as intended

Signed-off-by: Kamil Grabara <[email protected]>
@mafineeek
Copy link
Author

#292

Copy link
Member

@insertish insertish left a comment

Choose a reason for hiding this comment

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

The used invite should only be visible when the member is fetched by a moderator (gated by an appropriate permission, though I'm not sure which would be best).

@@ -41,6 +41,10 @@ auto_derived_partial!(
/// Time at which this user joined the server
pub joined_at: Timestamp,

/// Invite used by member to join server
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
pub used_invite: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

This would expose the used invite to any member of the server, which is unintended behaviour and could be abused.

Copy link
Author

Choose a reason for hiding this comment

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

Do we want to remove this field, or return Default in this case?

Copy link
Member

Choose a reason for hiding this comment

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

I mean if it were removed there's no way it would get to a client where it'd be useful, we would need to have some sort of permission (let's just suppose KickMembers for now) that it checks for when transforming members from database to API models.

Luckily for us, in most cases where we fetch members, we already check whether we can access the server (so permissions are already calculated), information about whether we have a specific permission (let's say KickMembers as before) can be passed through to where we transform the models which controls the visibility of the used_invite field (this can later be re-used for any other fields we might want to keep private to moderators).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🛑 Changes requested
Development

Successfully merging this pull request may close these issues.

2 participants