Discourse SSO updates #363
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was
linked to
issues
Dec 12, 2024
|
|
|
I hope you consider rewording this message to specify that this CLA is
the Developer Certificate of Origin. My initial reaction to this
notification was negative because I worried that this was one of those
CLAs corporations use to give the project owners special rights over
others' contributions.
…On Wednesday, Feb 5, 2025, 2:06 PM, wrote:
[](https://cla-assistant.io/PretendoNetwork/website?pullRequest=363) <br>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/PretendoNetwork/website?pullRequest=363) before we can accept your contribution.<br><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/PretendoNetwork/website?pullRequest=363) it.</sub> -- Reply to this email directly or view it on GitHub: #363 (comment) You are receiving this because you authored the thread. Message ID:
|
I hear your thoughts - and I agree the phrasing lends itself to worry people. Unfortunately, it's not something the CLA bot allows us to configure - so we'll just have to rely on people clicking on the CLA and seeing that it's just the certificate of origin |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
discourseUserExistsandsyncDiscourseSsofunctions 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.