Wip: SS Central - whitelist, discord link #946
Draft
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
Reviewer's Guide by SourceryThis PR implements Discord account linking via OAuth and a new whitelist system through SS Central. It removes client-side Discord ID lookup and verification, instead relying on SS Central for these functionalities. Sequence diagram for new Discord OAuth linking processsequenceDiagram
actor User
participant Client
participant SSCentral
participant Discord
User->>Client: Initiates Discord verification
Client->>SSCentral: Request verification token
SSCentral-->>Client: Return login URL with token
Client-->>User: Display login URL
User->>Discord: Authenticate via OAuth
Discord->>SSCentral: Confirm authentication
SSCentral-->>Client: Update Discord link status
Class diagram for SS Central subsystemclassDiagram
class SSCentral {
-list discord_links
+Initialize()
+load_whitelist()
+get_player_discord_async(client)
+is_player_whitelisted(ckey)
+add_to_whitelist(ckey, added_by, duration)
+whitelist_ban_player(player_ckey, admin_ckey, duration, reason)
}
class Interview {
+owner_ckey
+questions[]
+responses[]
+approve(client)
+deny(client)
+serialize_embed()
}
SSCentral -- Interview
note for SSCentral "New centralized system for
whitelist and Discord management"
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Furrior - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- The 'req' variable is undefined in this scope. This will cause runtime errors when logging responses. (link)
Overall Comments:
- The
make_blocking_request()implementation using UNTIL is dangerous and could cause server hangs/deadlocks. Consider making this fully asynchronous instead.
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 1 other issue
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @Furrior - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Hardcoded SS_CENTRAL_TOKEN found. (link)
Overall Comments:
- The blocking HTTP request using UNTIL is dangerous and could freeze the game if the request hangs. Consider using async requests consistently throughout the codebase.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Hey @Furrior - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Hardcoded SS_CENTRAL_TOKEN found. (link)
Overall Comments:
- The
make_blocking_request()method using UNTIL is dangerous and could cause deadlocks. Consider making this fully async or adding additional safeguards if blocking behavior is absolutely required.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Gaxeer
reviewed
Jan 1, 2025
|
This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself |
dj-34
added
RED LABEL
github-actions
bot
removed
the
🎸 Инструменты
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.
Что этот PR делает
fixes #191 #65 #66
Почему это хорошо для игры
Изображения изменений
Тестирование
Changelog
🆑
add: Привязка дискорда через OAuth
tweak: Новая система белого списка
/:cl:
Summary by Sourcery
Implement Discord link via OAuth and a new whitelist system that uses SS Central.
New Features:
Tests: