Repository navigation
Conversation
An AccountExternalLink links an account to a user of an external service like Discord, with a pending one-time code for the linking. Like the cash shop grants, it's a separate aggregate which refers to its account by id, so the external service can change it while a game server holds the account. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFZ9xErMB1EwkpfdUHzoZ
The chat command creates a one-time code, which the player enters in Discord. /discord unlink removes the link and publishes an AccountUnlinkedEvent, so that the Discord bot can remove the role of the user. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFZ9xErMB1EwkpfdUHzoZ
…er role Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFZ9xErMB1EwkpfdUHzoZ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFZ9xErMB1EwkpfdUHzoZ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USFZ9xErMB1EwkpfdUHzoZ
Deploying openmudocs with
|
| Latest commit: |
9cffa9a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://69112f90.openmudocs.pages.dev |
| Branch Preview URL: | https://claude-sweet-cray-w8crms-m4.openmudocs.pages.dev |
sven-n
left a comment
sven-n
left a comment
There was a problem hiding this comment.
Part of a review pass over the whole Discord stack. This is the security-sensitive one, so I went through the link flow in detail; it stands up well.
What I checked and found correct:
- Code strength. 8 characters from a 32-symbol alphabet is ~2^40, valid for 10 minutes, generated with
RandomNumberGenerator, stored only as a SHA-256 hash, and cleared on use. Confusable characters are excluded from the alphabet, andNormalizeCodemeans case and the separator don't matter on entry. Brute force isn't a practical concern at that size. - Uniqueness. The unique indexes on
(AccountId, Provider)and(Provider, ExternalUserId)are both there, so the "user already linked elsewhere" race inLinkAsynccan't produce duplicate rows — the second writer fails at the database instead. The second index relies on PostgreSQL treating NULLs as distinct so that multiple pending links coexist, which is what you want and is what it does. - Character ownership.
SelectCharacterAsyncverifiesGetAccountIdByCharacterNameAsync(characterName) == link.AccountId, so/charactercan only select characters of the linked account. This matters a lot later, because M6 keys game-master authority off the selected character. - Secrecy of the code. Both
DeferAsyncandFollowupAsyncpassephemeral: truefor the account commands, and the in-game side uses a blue message to the requesting player only.
Two comments inline. The admin-panel one is a real gap; the other is cosmetic.
One thing I'd consider as follow-up rather than a change here: nothing prunes CodeHash/CodeExpiresAt once a code expires unused, so stale hashes accumulate on rows that were never confirmed. Harmless (they can't be used, and the entropy makes them uninteresting even to someone reading the database), but a periodic cleanup would keep the table honest.
Generated by Claude Code
| this._isRemoving = true; | ||
| try | ||
| { | ||
| await new AccountLinkService(this.CreateContext).UnlinkAccountAsync(this._accountId, provider).ConfigureAwait(true); |
There was a problem hiding this comment.
This removes the row but doesn't publish AccountUnlinkedEvent, so it's the only one of the three unlink paths that leaves the user's Discord role in place.
The other two both clean up:
- in-game,
DiscordChatCommandPlugIn'sunlinkbranch publishesAccountUnlinkedEvent, whichDiscordBot.OnGameEventAsyncturns into a role removal; - from Discord,
DiscordAccountCommands.UnlinkAsyncreturns aDiscordRoleChangethatExecuteAccountCommandAsyncapplies.
So an admin who unlinks an account here — which is presumably the path used when a link is being removed against the user's wishes — leaves them holding the "Linked" role. From M5 on that also means keeping access to hosted guild channels until the next SyncHostedChannels tick happens to correct it.
UnlinkAccountAsync already returns the external user id for exactly this purpose, so it's a matter of publishing the event with it. The admin panel doesn't have an IEventPublisher to hand the way a game server context does, which is probably why this was skipped; if wiring one in is awkward, the alternative is to move the event publication into AccountLinkService.UnlinkAccountAsync so every caller gets it by construction.
While here: there's no confirmation prompt on the remove button, which is a little out of step with #978 adding confirmed deletion to the account list. Much lower stakes than deleting an account, since the user can just link again, so I'd call that optional.
Generated by Claude Code
| new AccountUnlinkedEvent(gameServerContext.Id, DateTime.UtcNow, AccountLinkService.DiscordProvider, userId)).ConfigureAwait(false); | ||
| } | ||
|
|
||
| await player.ShowLocalizedBlueMessageAsync(nameof(PlayerMessage.DiscordUnlinked)).ConfigureAwait(false); |
There was a problem hiding this comment.
This message is shown unconditionally, outside the if, so /discord unlink reports success even when there was nothing linked — UnlinkAccountAsync returned null and nothing happened.
Minor, but the player has no way to tell "I removed your link" from "there was no link", and the state-showing branch below does distinguish the two cases. Moving it into the if and adding an else that reuses PlayerMessage.DiscordNotLinked would make the three branches consistent.
Generated by Claude Code
This branch has not been deployed
2 participants
Milestone M4 of the Discord integration (#1102). Stacked on #1157 (M3).
What
AccountExternalLink, refers to its account by id like the cash shop grants. Fields: provider, external user id and name, character name, linked at, plus the hash and expiry of a pending one-time code. Unique(AccountId, Provider)and(Provider, ExternalUserId). Includes the EF migrationAddAccountExternalLink, regenerated persistence model classes and the compiled models ofEntityDataContext,AccountContextandTradeContext. The links are inIPlayerContext(EF and in-memory).AccountLinkService(GameLogic):ABCD-EFGH. It is valid for 10 minutes, stored as SHA-256, usable only once, and a new code invalidates the previous one. Entering it is case- and separator-insensitive./discordhas three forms:/discord linkshows a code,/discord unlinkremoves the link and publishes a newAccountUnlinkedEventgame event, and/discordshows the link./link <code>and/unlink; answers are ephemeral./character <name>selects another character of the same account (validated).linked(Linked Player) to the user in the routing guild./unlink, replacing a link, orAccountUnlinkedEventfrom the game remove the role again. The bot is now also anIGameEventListener./accounts/{login}/links, which shows the links and removes them (operator policy).Notes
Tests
AccountLinkServiceTest: linking, one-time use, expiry and replaced codes, relinking, unlinking from both sides, character selection.DiscordAccountCommandsTest: answers and role changes.DiscordChatCommandPlugInTest: the in-game command.Not tested: the migration wasn't applied to a real PostgreSQL database here, and nothing was run against a real Discord bot.
🤖 Generated with Claude Code
https://claude.ai/code/session_01USFZ9xErMB1EwkpfdUHzoZ
Generated by Claude Code