-
Notifications
You must be signed in to change notification settings - Fork 82
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
[Enhancement]: Add group syncing to Discourse SSO #345
Comments
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 |
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 |
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?
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. |
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 SSOThere 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
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 |
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.
Additionally, the subscription tier can be selected using |
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. |
Checked Existing
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:
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
andremove_groups
attributes instead of thediscourse connect overrides groups
setting, as that would break forum-only groups like category moderators.The text was updated successfully, but these errors were encountered: