Comment - FBXL Lotide

Comment on Granular group-based permissions for anonymous and logged in users

Can anyone help me understand how I need to adjust my theme components?

I tried to use the copy-post component as an example because I remember that it also uses a group setting that grants access to the feature. And that there was an issue because the “everyone” pseudo-group required a separate check, just like in my component, because comparing the IDs of the groups the user belongs to doesn’t help - those IDs need to be checked separately. That’s why I expected a recent change there, because as I understand it, the new groups are also pseudo-groups and the ID would need to be checked separately. Am I missing something that explains why this isn’t necessary here?

My favorite filters component has two group settings: one that allows groups to save their own filters, and one that offers standard filters.
By default, only members of the trust_level_0 group can use custom filters, because only registered users can have data stored in a custom user field. So here it would make sense if I didn’t allow anonymous_users as a selection. How do I do that in a theme component? Is there already an example for this?

The default setting for the default filters is “everyone,” because I find it helpful for even unregistered users to be able to see and use the default filters. The problem is that everyone changes to ‘logged_in_users’ even though I specifically selected it. Do I need to create a custom migration for this so that admins currently using everyone will continue to have filters for unregistered users in the future? When does this migration need to take place? Or does every admin need to change this individually after you have run the migration?

Is all this I’m worrying about actually unnecessary? If adjustments are needed, less than four weeks feels like a fairly short timeframe given the number of community-maintained components that could potentially be affected.
In addition to “copy-post,” I also looked at the unanswered filter component, but I couldn’t find any changes there either. It feels like I’m overlooking something important. After all, the change has been enabled by default for almost a week now. That’s why I assume that official components would already have been updated if adjustments were necessary.

source

Sort:hotnew top

martin@meta.discourse.org ⁨2⁩ ⁨days⁩ ago
Looking at these components, currentUser?.groups is not reliable anyway, since it only includes visible groups for the user, and the groups that they are in that impact permissions may not be serialized here:

github.com/discourse/discourse

app/serializers/current_user_serializer.rb

ba3e2d766

def groups owned_group_ids = GroupUser.where(user_id: id, owner: true).pluck(:group_id).to_set object .visible_groups .pluck(:id, :name, :has_messages) .map do |id, name, has_messages| group = { id: id, name: name, has_messages: has_messages } group[:owner] = true if owned_group_ids.include?(id) group end end

We get around this in core/plugins by doing stuff like this in the current user serializer:

github.com/discourse/discourse

app/serializers/current_user_serializer.rb

ba3e2d766

def can_delete_all_posts_and_topics object.in_any_groups?(SiteSetting.delete_all_posts_and_topics_allowed_groups_map) end

But obviously, this is not available to theme components/themes and their settings.

Image Moin:

The problem is that everyone changes to ‘logged_in_users’ even though I specifically selected it. Do I need to create a custom migration for this so that admins currently using everyone will continue to have filters for unregistered users in the future? When does this migration need to take place? Or does every admin need to change this individually after you have run the migration?

Hmm not sure, will have to think about this. If you truly meant everyone, then it would need to change to both logged_in_users AND anonymous_users . This was the main problem with everyone as stated in the OP – some people took it to mean only logged in users, others took it to mean logged in + anon, and it was very situation-dependent.

I chose the “only logged in users” interpretation because that was safer from a security POV.

Image Moin:

It feels like I’m overlooking something important. After all, the change has been enabled by default for almost a week now. That’s why I assume that official components would already have been updated if adjustments were necessary.

Nope, just didn’t think about theme components/themes and their settings and how they would be impacted by this change, I was mostly focused on site settings. Stuff like this especially will be very hard to find, since it’s not even using the AUTO_GROUPS const:

image

Anyway, I will think up some solutions to these issues, and I won’t move this change along to Stable until I figure them out.
source