website icon indicating copy to clipboard operation
website copied to clipboard

[Enhancement]: Add group syncing to Discourse SSO

Open MatthewL246 opened this issue 1 year ago • 10 comments

Checked Existing

  • [X] I have checked the repository for duplicate issues.

What enhancement would you like to see?

It would be convenient to have groups on the forum that are automatically synced based on account state.

This would include:

  • Developer (access level 3)
  • Juxtaposition moderator (access level 2)
  • Tester/supporter (access level 1)

It may also be possible to check if the PNID's linked Discord account has a moderator role in the server, which could be used to create a Discord moderator group.

Any other details to share? (OPTIONAL)

DiscourseConnect has support for adding and removing groups with SSO. It would probably be best to use the add_groups and remove_groups attributes instead of the discourse connect overrides groups setting, as that would break forum-only groups like category moderators.

MatthewL246 avatar Oct 05 '24 18:10 MatthewL246

Does this make sense as part of the SSO or as something we do via the API? The only flaw I see with this is that group syncing only happens during authentication, meaning that for someone to update their groups they would need to login again. I can see this causing some friction if someone's group needs to change while they are logged in (forcing them to logout and back in), and also if someone is logged out and their group changes (so, an admin being demoted), then it will look like they are still in said group(s) until they login again

jonbarrow avatar Oct 07 '24 04:10 jonbarrow

Also does Discourse support purely visual groups? Or do they have to be tied to an access level. It might be nice to have groups for stuff like donor status, like we have on the website

jonbarrow avatar Oct 07 '24 04:10 jonbarrow

The only flaw I see with this is that group syncing only happens during authentication, meaning that for someone to update their groups they would need to login again.

That's definitely a concern. The main pro to this method is that it would be very easy to implement. This could be mitigated by reducing the session timeout, but of course, making all users need to log in more often is just annoying. I agree that handling this by calling the Discourse API when the subscription status changes sounds more reasonable and addresses the flaw better than that. Handling it in SSO would still be necessary for current supporters who aren't changing their subscription status, though.

Handling developer/moderator status changes sounds like it would need to be in the admin panel?

Also does Discourse support purely visual groups?

Yes, you can create as many visual groups as you want without any ties to permissions. Donation tier group(s) could be added in addition to the tester/supporter group I mentioned. Groups also have some interesting visual features, like allowing group members to set the group name as their title or putting a small icon of the group logo next to their username.

MatthewL246 avatar Oct 07 '24 13:10 MatthewL246

Yes, you can create as many visual groups as you want without any ties to permissions. Donation tier group(s) could be added in addition to the tester/supporter group I mentioned. Groups also have some interesting visual features, like allowing group members to set the group name as their title or putting a small icon of the group logo next to their username.

Sorry I haven't seen this thread yet. I definitely agree with the idea to sync groups depending on role on website since it isn't clear for things like ban appeals that a user might be a staff member.

I don't really see an issue with syncing during authentication, just get the user to sign out and back in. That's already the case for updating profile pictures and it isn't really a hassle.

I don't really see an issue with syncing during authentication, just get the user to sign out and back in.

Personally, my main concern is more about people purposefully exploiting this fact to keep themselves in a group they shouldn't be in. For example, a dev/mod who has been demoted or a supporter who canceled their subscription.

MatthewL246 avatar Oct 13 '24 19:10 MatthewL246

I don't really see an issue with syncing during authentication, just get the user to sign out and back in.

Personally, my main concern is more about people purposefully exploiting this fact to keep themselves in a group they shouldn't be in. For example, a dev/mod who has been demoted or a supporter who canceled their subscription.

Ah of course, I didn't consider that possibility to be honest. Yeah, I completely agree with you here.

After reading the DiscourseConnect setup guide, I see a couple of potential solutions.

Manually syncing SSO

There is an API endpoint specifically for syncing SSO. This is pretty self-explanatory: send an API request with the SSO payload to manually sync it to the user.

Logging users out

Logging off users

You can use the POST admin endpoint /admin/users/{USER_ID}/log_out to log out any user in the system if needed.

To configure the endpoint Discourse redirects to on logout search for the logout redirect setting. If no URL has been set here you will be redirected back to the URL configured in discourse connect url.

Search users by external_id

User profile data can be accessed using the /users/by-external/{EXTERNAL_ID}.json endpoint. This will return a JSON payload that contains the user information, including the user_id which can be used with the log_out endpoint.

So, the website or admin panel could forcibly log out a user whenever they are removed from a sensitive group. This might be a simpler solution because it would prevent duplication of group-membership-handling code into multiple repos. All membership checks could be handled in the SSO login here, while everything else would just be a simple single API call to the log_out endpoint.

MatthewL246 avatar Nov 09 '24 01:11 MatthewL246

Since we have started to make more of an effort to separate different moderator types on Discord, this should also carry over to the forum. This means adding a few more groups and updating the group syncing logic.

  • Juxtaposition mods can be from the PNID's access level
  • Discord mods can be from the moderator role IDs, which should be possible using the existing Discord connection that's used to add supporter/tester roles
  • Network mods can use the same Discord role IDs method
  • I don't think it's necessary add the Discord VC mods

Additionally, the subscription tier can be selected using pnid.connections.stripe.tier_name or pnid.connections.stripe.tier_level.

MatthewL246 avatar Dec 10 '24 18:12 MatthewL246

  • I don't think it's necessary add the Discord VC mods

Regarding Discord VC mods, I agree with you to not add it but a generic Pretendo staff group could work too for that (and any potential future additions)

I'm not sure what the use of that would be. Being "staff" in general is less important than what specifically a staff member has control over, and I think it would be a little confusing to have someone who is only "generic staff." It could definitely be added if there was a specific use for it.

MatthewL246 avatar Dec 10 '24 18:12 MatthewL246