Allow anonymous users to inherit their parent user groups

feature
,
1

Hello!

Could I please get a review for this PR?

We’re trying to create a region-specific discourse, where a user’s membership will limit them to certain groups, but currently this conflicts with anonymous user support because anonymous users do not have any group associations.

This PR adds groups association to anonymous users.

1 Like
2

Have you considered the increased risk of deanonymisation attacks that this would allow?

2 Likes
3

Thanks for raising this topic!

I have considered deanonymisation attacks and you’re right - user group inheritance is not a tool for everyone and is especially vulnerable for instances with high cardinality group affiliations. As detailed in the PR, the original concept to support anonymous posting in regional groups, which will be implicitly low-cardinality in our instance.

edit: I’ve added a group attribute called anonymous_user_inheritance, which allows admins to choose whether or not the group gets inherited by anonymous users. I’ve also added a warning next the admin setting to enable the feature, and I’m open to any further suggestions.

1 Like
4

Is this the right Category to request PR reviews or should I crosspost into a different Category?

Thank you!

1 Like
5

There isn’t really a category to request code reviews, as such, so this is likely the best one for it.

PRs are often welcome, though it’s good to confirm that it’s a direction/feature that Discourse wants to pursue before diving too deep.

Alternatively, you could look at introducing this feature in a plugin so it’s available to use without folding into the main code.

2 Likes
7

Thanks for the tip! A plugin would actually suit our uses well, except that this change requires a database migration. Would a migration be feasible on a plugin?

1 Like
8

Yes, you can run database migrations in a plugin.

1 Like
9

Yes, but IMO it’s bad practice to modify core tables in a plugin.

If you move this to a plugin, it’s better to use group_custom_fields or your own, designated, table.

In this specific case you could also (instead) add a site setting like “allowed inherited groups” and have the admin select all the groups that can be inherited into this single setting. That would keep things much more simple and it groups all the functionality provided by the plugin into a single place.

5 Likes
11

Thanks for the feedback Richard! Could you help me understand your second proposal?

have the admin select all the groups that can be inherited into this single setting

To verify my understanding, your suggestion is to have the general site setting to allow group inheritance, and then on each individual group’s settings page, there would be a checkbox as to whether this group is inheritable or not, so it’s similar to what I have right now, except the redundant setting is removed.

have the admin select all the groups that can be inherited into this single setting
Were you suggesting the above, or did you have something else in mind? My original idea was to build a sort of selection tool where there’s a checkbox menu or something, but that become implementationally complex very fast.

Thanks for the tip on plugins too! I’ll consider the plugin option in future contributions. :grinning:

1 Like
12

No, I’m suggesting that you don’t make this a setting on the group page, but one site wide setting that contains all the groups that are inheritable instead.

A similar setting is Admin - Settings - Users - anonymous posting allowed groups. It’s not a checkbox on every groups settings page “allow anonymous posting for this group”, it’s one setting with all the groups that allow anonymous posting,

or Admin - Settings - Posting - here mention allowed groups instead of a checkbox on every groups settings page “allow here mentions for this group”.

It’s easier to implement, it’s less database clutter and it provides a better oversight to the forum administrator.

2 Likes