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

Discourse SSO updates #363

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Discourse SSO updates #363

wants to merge 4 commits into from

Conversation

MatthewL246
Copy link
Member

Resolves #345 and #352

Changes

This adds new features to the Discourse SSO that will sync a user's groups based on their PNID's access level, Stripe tier, and linked Discord account's roles in the server. The access levels, tier levels, role IDs, and group names are all controlled by values in the config file for easy modification.

This also adds a new feature to manually sync a user's SSO state without requiring them to sign out and back in. It is currently used after handling a Stripe event, and it could be used in the future when a user is promoted/demoted as a moderator/developer. This would address the concerns shared in #345 over a user purposefully staying signed in to avoid being removed from a privileged group. For now, I think using this after handling a Stripe event and having a forum admin manually remove a user's groups when they are demoted will work fine, considering how rarely the second event happens.

Setting up the manual sync feature will require creating a Discourse API key. I would suggest setting its user to system, permissions to granular, and only enabling the sync SSO route for security reasons.

Prerequisites

This requires PretendoNetwork/account#128 to be merged first.

Current issues

As I commented in createDiscoursePayload, I noticed some issues with primary groups while testing this. I wouldn't consider them showstoppers because this basically only affects developers and moderators, none of a user's groups will change during most logins, and users can select their own primary group in their profile settings.

Testing

I tested this as thoroughly as I could using self-hosted instances of the website (set up with a real Discord application and bot) and Discourse. The only issue is the Stripe integration, as I don't have a Stripe account. I tested the Stripe tiers functionality by creating fake Stripe tiers in the database under the assumption that they are just regular integers. I also manually tested the discourseUserExists and syncDiscourseSso functions used in the Stripe event handling by using the debug console. However, adding the SSO syncing to the end of 'handleStripeEvent` is a bit of an assumption and should be reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant