From 141c9b82715e0a8979b43d01e44cd935389a1a73 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 13:36:20 -0400 Subject: [PATCH 01/10] docs(adr): validate PLAN-0005/ADR-0005 against post-ADR-0006/0008 architecture PLAN-0005 was written before the Host->Hosting+Adapters.* split (ADR-0006) and the Ruleset.Rpg extraction (ADR-0008) - it referenced src/SharpMud.Host, src/SharpMud.Ruleset.Classic, TelnetHostContext, and HostRunner, none of which exist anymore. Re-verified every file reference against the current codebase and corrected them, added the registration call site ADR-0008 introduced (SharpMud.Ruleset.Rpg's ServiceCollectionExtensions.cs), and confirmed IThingRepository is Engine-owned so the admin commands' planned location is still architecturally sound. Also simplifies the SHARPMUD_INITIAL_ADMIN bootstrap: the original two-checkpoint design (a boot-time Program.cs check plus a separate MaybeCreateAsync check) existed because LoginFlow used to be manually parameter-threaded through a per-connection context object. LoginFlow is DI-constructed now, so both cases collapse into one check inside LoginFlow.RunAsync itself, covering fresh-server, restart, and late-set-env-var cases identically. ADR-0005 gets a correction note (not a rewrite - Decision/Context stay untouched per this repo's ADR-immutability convention) recording why the file paths changed and pointing at the plan for the simplified mechanism. Co-Authored-By: Claude Sonnet 5 --- ...rity-role-model-and-moderation-commands.md | 32 ++ ...rity-role-model-and-moderation-commands.md | 431 ++++++++++-------- 2 files changed, 265 insertions(+), 198 deletions(-) diff --git a/docs/adr/0005-security-role-model-and-moderation-commands.md b/docs/adr/0005-security-role-model-and-moderation-commands.md index 278b4b0..c8abb77 100644 --- a/docs/adr/0005-security-role-model-and-moderation-commands.md +++ b/docs/adr/0005-security-role-model-and-moderation-commands.md @@ -6,6 +6,38 @@ **Decision Makers:** solo (design dive conducted with the user) +**Correction (2026-07-23, before implementation started):** the file paths +and call chain this ADR's Decision Outcome/bootstrap section describe +(`src/SharpMud.Host`, `TelnetHostContext`, `HostRunner`, +`src/SharpMud.Ruleset.Classic`, `ClassicCommands.RegisterAll`) reflect the +architecture as of this ADR's acceptance date — that architecture no longer +exists. [ADR-0006](0006-nuget-package-distribution.md) (accepted and +implemented before this slice's implementation began) split `SharpMud.Host` +into `SharpMud.Hosting` (DI composition helpers) + `SharpMud.Adapters.*` +(transport, e.g. `TelnetTransportBackgroundService` replacing `HostRunner`) ++ a consumer composition root (now `samples/SharpMud.Samples.Classic +/Program.cs`, not a project called `SharpMud.Host`), and moved ruleset +content out of a packaged `SharpMud.Ruleset.Classic` into that same +unpackaged sample. [ADR-0008](0008-ruleset-scaffolding-tier.md) later +extracted the reusable combat scaffolding from that sample into +`SharpMud.Ruleset.Rpg`. None of this changes what was decided here +(`SecurityRole`, the Decorator pattern, `RegisterOpen`/`RegisterWithRole`, +role accumulation, the bootstrap env var) — only where the mechanism's +pieces physically live, which is why this is a correction note rather than +a supersession. The bootstrap mechanism specifically is also simplified in +the implementation (see [PLAN-0005](../plans/0005-security-role-model-and-moderation-commands.md)): +`LoginFlow` is DI-constructed now (it wasn't, structurally, when this ADR +was written — there was no `TelnetHostContext` to thread a value through), +so the original two-separate-checkpoints design (a boot-time Program.cs +check plus a `MaybeCreateAsync` check) collapses to one check inside +`LoginFlow` itself, run against whichever `Thing` it's about to return +(new or existing) — same outcome (idempotent grant, covers fresh-server and +restart cases), fewer moving parts. Corrected here rather than superseded +because the actual decision - what gets built and why - is unchanged; only +implementation-mechanism detail moved, and that detail belongs in the plan +per this repo's own ADR/plan split, not repeated (and inevitably going +stale again) here. + ## Context Per ADR-0001, this is Slice 3 of the WheelMUD reconciliation roadmap. diff --git a/docs/plans/0005-security-role-model-and-moderation-commands.md b/docs/plans/0005-security-role-model-and-moderation-commands.md index 2894efd..252c6b2 100644 --- a/docs/plans/0005-security-role-model-and-moderation-commands.md +++ b/docs/plans/0005-security-role-model-and-moderation-commands.md @@ -4,7 +4,29 @@ **Status:** Not Started -**Last updated:** 2026-07-17 +**Last updated:** 2026-07-23 + +**Validated against current architecture (2026-07-23), before starting +implementation:** this plan was originally written against the +pre-[ADR-0006](../adr/0006-nuget-package-distribution.md) project layout +(`src/SharpMud.Host`, `src/SharpMud.Ruleset.Classic`, `TelnetHostContext`, +`HostRunner`). None of those exist anymore — `SharpMud.Host` split into +`SharpMud.Hosting` + `SharpMud.Adapters.*`, the composition root is +`samples/SharpMud.Samples.Classic/Program.cs`, `HostRunner` became +`TelnetTransportBackgroundService`, and there's no per-connection context +object (dependencies resolve via `IServiceProvider` per connection). +[ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) further extracted +combat scaffolding into `SharpMud.Ruleset.Rpg`, adding a second +`registry.Register(...)` call site (`AttackCommand`/`FleeCommand`) this +plan's registry-migration task needs to cover too. Every task/file +reference below has been re-verified against the actual current code, not +just find-and-replaced — see ADR-0005's own correction note for the +mechanism-level change (bootstrap collapses from two checkpoints to one, +now that `LoginFlow` is DI-constructed rather than manually +parameter-threaded through a context object that no longer exists). The +underlying decision (`SecurityRole`, the Decorator pattern, +`RegisterOpen`/`RegisterWithRole`, role accumulation, the 8 commands, the +bootstrap env var) is unchanged. ## Goal @@ -60,13 +82,34 @@ an open item below. inner `ICommand`, checks the *actor's own* `IsMuted` (not a target's — this gates the muted player's own `say`/`emote`, not something they're doing to someone else) before delegating. -- [ ] `ICommandRegistry.cs` / `CommandRegistry.cs`: remove `Register - (ICommand)` from the public surface; add `RegisterOpen(ICommand)` - and `RegisterWithRole(ICommand, SecurityRole)` (the latter wraps in +- [ ] `ICommandRegistry.cs` / `CommandRegistry.cs` + (`src/SharpMud.Engine/Commands/`): remove `Register(ICommand)` from + the public surface; add `RegisterOpen(ICommand)` and + `RegisterWithRole(ICommand, SecurityRole)` (the latter wraps in `RoleGuardedCommand` before storing). -- [ ] Update every existing registration call site (`BuiltinCommands - .RegisterAll`, `ClassicCommands.RegisterAll`) from `Register` to - `RegisterOpen` — mechanical, no behavior change. +- [ ] Update every existing registration call site from `Register` to + `RegisterOpen` — mechanical, no behavior change. Two call sites + today, not one (verified by grep, not assumed): + - [ ] `src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs` — all + 17 registrations. While here: wrap the `SayCommand`/ + `EmoteCommand` registrations specifically in + `MuteGuardedCommand` before `RegisterOpen` (mute enforcement is + an `Engine`-level concern per ADR-0005, and `BuiltinCommands + .RegisterAll` runs for every consumer via `Hosting`'s + `AddSharpMudRuleset(...)`, so wrapping here — not per-consumer + — is what actually makes mute universal). + - [ ] `src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs` — the + `AttackCommand`/`FleeCommand` registrations inside + `AddSharpMudRpgRuleset(...)`. This call site didn't exist when + ADR-0005 was accepted ([ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) + added it afterward) — `attack`/`flee` aren't role-gated, just + migrated to the new intentional entry point like everything + else. + - [ ] `samples/SharpMud.Samples.Classic/ClassicCommands.cs` no longer + exists (removed when [ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) + landed — it had nothing left to register) — do not recreate it + for this plan; the new admin commands register through their + own `AdminCommands.RegisterAll`, see below. - [ ] `HelpCommand.cs`: filter `registry.Commands` by the actor's roles before listing them — caught in self-review. `RoleGuardedCommand` passes `Verb`/`Aliases` straight through from the wrapped command, so @@ -79,12 +122,12 @@ an open item below. ### `PlayerBehavior` + persistence + `SessionLoop` -- [ ] `PlayerBehavior.cs`: add `SecurityRole Roles { get; private set; } = - SecurityRole.Player`, `bool IsMuted { get; private set; }`, `bool - IsBanned { get; private set; }`, plus mutation methods - (`GrantRole`/`RevokeRole`, `Mute`/`Unmute`, `Ban`/`Unban`) rather than - public setters, matching `ConnectionState`'s existing - transition-method style. +- [ ] `src/SharpMud.Engine/Behaviors/PlayerBehavior.cs`: add `SecurityRole + Roles { get; private set; } = SecurityRole.Player`, `bool IsMuted { + get; private set; }`, `bool IsBanned { get; private set; }`, plus + mutation methods (`GrantRole`/`RevokeRole`, `Mute`/`Unmute`, + `Ban`/`Unban`) rather than public setters, matching + `ConnectionState`'s existing transition-method style. - [ ] Also add `bool WasBooted { get; private set; }` (transient, like `Session`/`ConnectionState` — `Ignore`d in `PlayerBehaviorConfiguration`) and a `MarkBooted()` method. @@ -102,13 +145,14 @@ an open item below. `PlayerBehavior` *before* calling `DisconnectAsync`; the target's own `SessionLoop.RunAsync` (a completely separate call stack) checks it in its `finally` block. - - [ ] `SessionLoop.cs`: in the `finally` block, treat `WasBooted` - exactly like `explicitQuit` — both mean "this disconnect was - intentional, skip `Linkdead` and remove immediately" (same - save-then-remove ordering `explicitQuit` already uses, not - `EnterLinkdead`'s mutate-before-save ordering). Concretely: - replace the bare `explicitQuit` checks in both branches with - `explicitQuit || (playerBehavior?.WasBooted ?? false)`. + - [ ] `src/SharpMud.Hosting/SessionLoop.cs`: in the `finally` block, + treat `WasBooted` exactly like `explicitQuit` — both mean + "this disconnect was intentional, skip `Linkdead` and remove + immediately" (same save-then-remove ordering `explicitQuit` + already uses, not `EnterLinkdead`'s mutate-before-save + ordering). Concretely: replace the bare `explicitQuit` checks + in both branches with `explicitQuit || (playerBehavior + ?.WasBooted ?? false)`. - [ ] `GrantRole(SecurityRole role)`: ORs in `role` *and* every tier it implies (`FullAdmin` → also `MinorAdmin` + `Player`; `FullBuilder` → also `MinorBuilder`) per ADR-0005's @@ -132,42 +176,36 @@ an open item below. normal, directly-user-triggerable business-rule outcome (an admin typed a `rolerevoke` that doesn't make sense given the target's current roles), not a bug or an invariant violation, - so it must be a return value the caller checks (caught in PR - review — the original wording said "throw/reject," which - contradicts the standard). Signature: + so it must be a return value the caller checks. Signature: `string? RevokeRole(SecurityRole role)` — `null` on success, a message naming the blocking higher tier on failure ("still has FullAdmin, which includes MinorAdmin — revoke FullAdmin - instead"). Corrected during self-review: earlier wording cited - this as mirroring "`MoveRequest.CancelReason`," but no - `MoveRequest` type exists — the actual precedent - (`UseExitEvent.CancelReason`, `src/SharpMud.Engine/Core - /Events.cs`) is a *property* set via `.Cancel(reason)` on a - published cancellable event, a different shape (pub/sub - event-object mutation, not a direct method return). A plain - nullable-string return is simpler and doesn't need that - machinery here — it just needs to be a return value, per - `coding-standards.md`, not a citation to a nonexistent type. - `RoleRevokeCommand` relays a non-null return straight to the - admin; never wraps this call in a try/catch. -- [ ] `PlayerBehaviorConfiguration.cs`: map `Roles` with the plain-enum - default EF conversion (matching `WearableBehaviorConfiguration`'s - `Slot` precedent — no custom value converter needed); map `IsMuted`/ - `IsBanned` as plain persisted columns (NOT `Ignore`d — unlike - `ConnectionState`, these must survive a restart). `Ignore(x => - x.WasBooted)` alongside `Session`/`ConnectionState` — transient, - same category, never meaningful across a restart. + instead"). `RoleRevokeCommand` relays a non-null return + straight to the admin; never wraps this call in a try/catch. +- [ ] `src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs`: + map `Roles` with the plain-enum default EF conversion (matching + `WearableBehaviorConfiguration`'s `Slot` precedent — no custom value + converter needed); map `IsMuted`/`IsBanned` as plain persisted + columns (NOT `Ignore`d — unlike `ConnectionState`, these must + survive a restart). `Ignore(x => x.WasBooted)` alongside `Session`/ + `ConnectionState` — transient, same category, never meaningful + across a restart. ### Moderation commands (`src/SharpMud.Engine/Commands/Builtin/Admin/`) -`Mute`/`Unmute`/`Ban`/`Unban`/`RoleGrant`/`RoleRevoke` need `IThingRepository` -for offline target lookup + immediate saves — `CommandContext` only carries -`World`/`Session`, not the repository, and today the repository is only -available inside `SessionLoop`. Rather than extending `CommandContext` -(bigger blast radius, touches every command's context), these six commands -take `IThingRepository` via their own constructor — the same shape -`ClassicCommands.RegisterAll` already uses for `combatManager`/`random`. -`Boot`/`Announce` don't need it (online-only / broadcast-only). +`Mute`/`Unmute`/`Ban`/`Unban`/`RoleGrant`/`RoleRevoke` need +`IThingRepository` for offline target lookup + immediate saves — +`CommandContext` only carries `World`/`Session`, not the repository. Rather +than extending `CommandContext` (bigger blast radius, touches every +command's context), these six commands take `IThingRepository` via their +own constructor — same shape `SharpMud.Ruleset.Rpg`'s `AttackCommand`/ +`FleeCommand` already use for their own dependencies. **`IThingRepository` +is defined in `SharpMud.Engine` itself** (`src/SharpMud.Engine/Core +/IThingRepository.cs` — `SharpMud.Persistence` implements it, not the +reverse), so these commands living in `SharpMud.Engine` alongside it is not +a layering violation — verified directly against the current dependency +graph, not assumed. `Boot`/`Announce` don't need it (online-only / +broadcast-only). **"Online" means `ConnectionState == Playing` *and* `Session is { IsConnected: true }` — both, not either alone.** `WhoCommand`'s iteration @@ -195,7 +233,8 @@ null-session disconnect/write. The fix: use the exact same combined check `LoginFlow.LoginExistingAsync` already established for this (`playerBehavior.ConnectionState == ConnectionState.Playing && playerBehavior.Session is { IsConnected: true }`) — don't invent a -narrower one. +narrower one. Verified this check still reads exactly this way in the +current `src/SharpMud.Hosting/LoginFlow.cs`. - [ ] `BootCommand` (`MinorAdmin`, no repository dependency) — disconnects a currently-online (`ConnectionState == Playing && Session is { @@ -204,11 +243,10 @@ narrower one. with a null/disconnected `Session`). **Calls `target.FindBehavior()!.MarkBooted()` before** `target's session.DisconnectAsync(...)` — without this the boot is - cosmetic (caught in PR review; see the `PlayerBehavior`/ - `SessionLoop` task above for why). Writes a message to the target's - session first (mirrors `QuitCommand`'s "Goodbye!" before - disconnecting), e.g. "You have been disconnected by an - administrator." + cosmetic (see the `PlayerBehavior`/`SessionLoop` task above). Writes + a message to the target's session first (mirrors `QuitCommand`'s + "Goodbye!" before disconnecting), e.g. "You have been disconnected + by an administrator." - [ ] `MuteCommand`/`UnmuteCommand` (`MinorAdmin`, `IThingRepository`) — sets/clears `IsMuted` on a target (online-or-not, mirrors `LoginFlow`'s live-then-repository lookup), saves immediately. @@ -221,101 +259,107 @@ narrower one. online-or-not lookup, saves immediately. **If the target is currently online (`ConnectionState == Playing && Session is { IsConnected: true }`), also disconnects them the same way - `BootCommand` does** — `MarkBooted()` then session write + `Disconn - ectAsync(...)` (caught in PR review: `SessionLoop` never re-checks - `IsBanned` mid-session, so without this an already-connected banned - player keeps issuing commands until an admin separately remembers - to `boot` them). **Rejects self-targeting** - (caught in self-review: `Ban` has no in-game recovery — - `SHARPMUD_INITIAL_ADMIN` only re-grants roles, it doesn't clear - `IsBanned` — so an admin banning themselves is locked out short of a - manual DB edit; `boot`/`mute` self-targeting is left alone, both are - harmless and trivially reversible by the same admin). `UnbanCommand` - needs no such guard (undoing your own ban isn't reachable — you - can't be logged in while banned). + `BootCommand` does** — `MarkBooted()` then session write + + `DisconnectAsync(...)` (`SessionLoop` never re-checks `IsBanned` + mid-session, so without this an already-connected banned player + keeps issuing commands until an admin separately remembers to + `boot` them). **Rejects self-targeting** (`Ban` has no in-game + recovery — `SHARPMUD_INITIAL_ADMIN` only re-grants roles, it doesn't + clear `IsBanned` — so an admin banning themselves is locked out + short of a manual DB edit; `boot`/`mute` self-targeting is left + alone, both are harmless and trivially reversible by the same + admin). `UnbanCommand` needs no such guard (undoing your own ban + isn't reachable — you can't be logged in while banned). - [ ] `RoleGrantCommand`/`RoleRevokeCommand` (`FullAdmin`, `IThingRepository`) — mutates a target's `Roles` via `GrantRole`/`RevokeRole` (accumulation/hierarchy-invariant enforcement happens inside `PlayerBehavior` itself, not here), online-or-not lookup, saves immediately. Validate the role name argument against an **explicit allowlist of individually-grantable - roles — not just "is this a real `SecurityRole` name."** Caught in - PR review: a plain `Enum.TryParse` would also accept - `All` (every current *and future* flag — not a real assignable - tier, a severe over-grant) and `None` (a meaningless no-op - sentinel), since both are literally named enum members. Reject - either with a clear message rather than silently persisting them. - **`RoleRevokeCommand` rejects revoking your own `FullAdmin`** (caught - in self-review — same class of lockout risk as self-`Ban`: a sole - `FullAdmin` revoking their own tier has no in-game path back without - another `FullAdmin` already present to re-grant it). Revoking any - other role from yourself, or revoking `FullAdmin` from someone - *else*, is unaffected. `RoleRevokeCommand` checks `RevokeRole`'s - `string?` return (not a - caught exception — see the `PlayerBehavior` task above) and relays - a non-null failure straight to the admin as the rejection message. + roles — not just "is this a real `SecurityRole` name."** A plain + `Enum.TryParse` would also accept `All` (every current + *and future* flag — not a real assignable tier, a severe + over-grant) and `None` (a meaningless no-op sentinel), since both + are literally named enum members. Reject either with a clear + message rather than silently persisting them. **`RoleRevokeCommand` + rejects revoking your own `FullAdmin`** (same class of lockout risk + as self-`Ban`: a sole `FullAdmin` revoking their own tier has no + in-game path back without another `FullAdmin` already present to + re-grant it). Revoking any other role from yourself, or revoking + `FullAdmin` from someone *else*, is unaffected. `RoleRevokeCommand` + checks `RevokeRole`'s `string?` return (not a caught exception) and + relays a non-null failure straight to the admin as the rejection + message. - [ ] Register all 8 via `RegisterWithRole` in a new - `AdminCommands.RegisterAll(registry, repository)` (mirrors - `BuiltinCommands`/`ClassicCommands`'s shape — `ClassicCommands - .RegisterAll` already takes extra constructed dependencies the same - way), called from `Program.cs` alongside the existing `RegisterAll` - calls, passing the already-constructed `repository`. Wrap `say`/ - `emote`'s existing registrations in `MuteGuardedCommand` at the same - call site. + `AdminCommands.RegisterAll(registry, repository)` + (`src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommands.cs` — + mirrors `BuiltinCommands`'s shape). **Wiring point, corrected from + the pre-ADR-0006 plan**: there's no monolithic `Program.cs` + `RegisterAll` sequence to append to anymore — `Hosting`'s + `AddSharpMudRuleset(...)` already auto-calls `BuiltinCommands + .RegisterAll` and then invokes a single consumer callback (calling + it, or the underlying `AddSharpMudRuleset`, a second time + independently would silently clobber the first registration per + ADR-0008's Decision Outcome). Call `AdminCommands.RegisterAll` from + inside the `registerConsumerCommands` callback already passed to + `AddSharpMudRpgRuleset(...)` in + `samples/SharpMud.Samples.Classic/Program.cs`, resolving + `IThingRepository` from that callback's own `IServiceProvider` + (`sp.GetRequiredService()`) — same pattern + `SharpMud.Ruleset.Rpg`'s own `ServiceCollectionExtensions.cs` uses + to resolve its own dependencies inside that callback shape. ### Login-flow + bootstrap +**Design simplified from the original two-checkpoint plan** (see ADR-0005's +correction note) — `LoginFlow` is a DI-constructed singleton today (`src +/SharpMud.Hosting/LoginFlow.cs`, resolved via `_serviceProvider +.GetRequiredService()` per connection in `TelnetTransportBackground +Service.HandleConnectionAsync`), not manually parameter-threaded through a +per-connection context object — there is no `TelnetHostContext`/ +`HostRunner` to thread a value through anymore. That makes a single +in-`LoginFlow` check both simpler and more correct than the original +two-separate-checkpoints design: it runs for every login, whether the +character is brand-new (`MaybeCreateAsync`) or pre-existing +(`LoginExistingAsync`), covering the fresh-server case, the restart case, +and "admin logs in later after the env var was set," all identically — +no separate boot-time code path to keep in sync with the login-time one. + - [ ] `LoginFlow.LoginExistingAsync`: after password verification succeeds, check `IsBanned` → reject with a distinct message before the `ConnectionState` branch. -- [ ] `HostOptions.cs`: add `string? InitialAdminUsername`, parsed from - `SHARPMUD_INITIAL_ADMIN`. -- [ ] `Program.cs`: add `["SHARPMUD_INITIAL_ADMIN"] = - Environment.GetEnvironmentVariable("SHARPMUD_INITIAL_ADMIN")` to the - `env` dictionary built before `HostOptions.Parse(args, env)` — caught - in PR review. `Program.cs` builds a **fixed** dictionary of only the - three existing env vars (`SHARPMUD_MODE`/`SHARPMUD_TELNET_PORT`/ - `SHARPMUD_DB_PATH`), not a full environment pass-through, so adding - the parse logic to `HostOptions.cs` alone isn't enough — - `SHARPMUD_INITIAL_ADMIN` needs its own entry here or the real host - never sees it, even with the env var actually set in production. -- [ ] Bootstrap the grant in **two** places, not just one — a boot-time-only - check is a no-op on a genuinely fresh server, since the target - character doesn't exist yet at boot and only gets created later - through the normal login flow (caught in PR review): - - [ ] `Program.cs`: after world load/build, if `InitialAdminUsername` - is set and that username's character already exists (live or - via repository — the "restart of an existing world" case), - idempotently `GrantRole(FullAdmin)` + save if not already - present. - - [ ] `LoginFlow.MaybeCreateAsync`: after a new character is created - and saved, if its username matches `InitialAdminUsername`, - `GrantRole(FullAdmin)` + save again (the fresh-server case). - **This needs `InitialAdminUsername` threaded all the way down - to `MaybeCreateAsync` — not hidden global state** (caught in - PR review): the actual Telnet call chain is - `HostRunner.RunTelnetAsync` → `HandleConnectionAsync` → - `LoginFlow.RunAsync(session, context.World, context.Repository, - context.StartingRoom, ct)` → `MaybeCreateAsync`, and neither - `TelnetHostContext` nor `LoginFlow.RunAsync`'s signature - carries `HostOptions`/`InitialAdminUsername` today. Concretely: - - [ ] `TelnetHostContext` (`src/SharpMud.Host - /TelnetHostContext.cs`) gains a `string? - InitialAdminUsername` field. - - [ ] `LoginFlow.RunAsync`/`MaybeCreateAsync` gain an - `InitialAdminUsername` parameter (already at/near the - 4-param limit — a small parameter object may be - warranted here too, matching `TelnetHostContext`'s own - precedent per `coding-standards.md`'s 4-param rule, - rather than pushing a 5th positional parameter through). - - [ ] `HostRunner.HandleConnectionAsync` passes - `context.InitialAdminUsername` through to - `LoginFlow.RunAsync`. - - [ ] `Program.cs`'s `TelnetHostContext` construction passes - `hostOptions.InitialAdminUsername`. - Both paths call the same `PlayerBehavior.GrantRole(SecurityRole - .FullAdmin)`, so both get the accumulation behavior (also granting - `MinorAdmin`/`Player`) for free. +- [ ] `src/SharpMud.Hosting/SharpMudHostOptions.cs`: add `string? + InitialAdminUsername`, parsed from `SHARPMUD_INITIAL_ADMIN` in + `SharpMudHostOptions.Parse(env)`. +- [ ] `samples/SharpMud.Samples.Classic/Program.cs`: add + `["SHARPMUD_INITIAL_ADMIN"] = Environment.GetEnvironmentVariable + ("SHARPMUD_INITIAL_ADMIN")` to the `env` dictionary already built + before `SharpMudHostOptions.Parse(env)` — that dictionary is a + **fixed** set of the vars this sample actually reads (currently just + `SHARPMUD_DB_PATH`), not a full environment pass-through, so the new + var needs its own entry here or the real host never sees it even + with it set in production. +- [ ] `builder.Services.AddSingleton(hostOptions);` in the same + `Program.cs`, right after `SharpMudHostOptions.Parse(env)` — **new + requirement this plan's implementer needs that the original didn't**: + `SharpMudHostOptions` isn't registered in DI today (`Program.cs` + only ever uses it as a local value, e.g. `hostOptions.DbPath` passed + directly into `AddSharpMudSqlitePersistence(...)`), but `LoginFlow` + needs to constructor-inject it now, so it has to actually be in the + container. +- [ ] `LoginFlow`: constructor-inject `SharpMudHostOptions` (a 4th + dependency, alongside `WorldContext`/`IThingRepository`/ + `IPlayerFactory`). In `RunAsync`, after `LoginExistingAsync`/ + `MaybeCreateAsync` produces a non-null `player`, before returning + it: if `_hostOptions.InitialAdminUsername` is set and case- + -insensitively equals that player's `PlayerBehavior.Username`, and + they don't already hold `FullAdmin`, call `GrantRole(FullAdmin)` + (which accumulates `MinorAdmin`/`Player` too, per the accumulation + rule) and `SaveTreeAsync` immediately — same "persist immediately, + don't wait for disconnect" reasoning `MaybeCreateAsync` already + applies to a freshly-created character. Idempotent by construction + (the `FullAdmin` check), so safe to run on every login for that + username, not just the first. ### Docs @@ -325,16 +369,16 @@ narrower one. as current state, update the existing "deferred moderation tooling" forward-reference to point at ADR-0005/this plan. - [ ] `docs/deployment.md`: add `SHARPMUD_INITIAL_ADMIN` to the Runtime - Configuration table (caught in PR review — this table is the - documented list of every `HostOptions` env var, and + Configuration table — this table is the documented list of every + `SharpMudHostOptions`/sample-level env var, and `SHARPMUD_INITIAL_ADMIN` is the *only* bootstrap path to a `FullAdmin` in a fresh deployment; omitting it here means deploying - a container with no discoverable way to administer it). + a container with no discoverable way to administer it. - [ ] `SPEC.md`: update the "Moderation/admin tooling" Deferred/Open Item to reflect what's actually implemented vs. still deferred (Find/ GoTo/Control/Clone/Spawn/Jail/Buff/Relinquish, audit logging). - [ ] `docs/adr/README.md` / `docs/plans/README.md`: index rows for - ADR-0005/PLAN-0005. + ADR-0005/PLAN-0005 flip to `Accepted`/in-progress-then-`Done`. - [ ] `docs/plans/0001-wheelmud-reconciliation-roadmap.md`: check off Slice 3. @@ -356,14 +400,18 @@ New: - Matching test files under `tests/SharpMud.Engine.Tests/Commands/...` Modified: -- `src/SharpMud.Engine/Commands/ICommandRegistry.cs`, - `CommandRegistry.cs`, `Builtin/HelpCommand.cs` -- `src/SharpMud.Engine/Commands/BuiltinCommands.cs` (or wherever - `RegisterAll` lives), `src/SharpMud.Ruleset.Classic/ClassicCommands.cs` +- `src/SharpMud.Engine/Commands/ICommandRegistry.cs`, `CommandRegistry.cs`, + `Builtin/HelpCommand.cs`, `Builtin/BuiltinCommands.cs` +- `src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs` (`Register` → + `RegisterOpen` for `AttackCommand`/`FleeCommand` — call site added by + ADR-0008, after this plan was first written) - `src/SharpMud.Engine/Behaviors/PlayerBehavior.cs` - `src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs` -- `src/SharpMud.Host/LoginFlow.cs`, `HostOptions.cs`, `Program.cs`, - `HostRunner.cs`, `TelnetHostContext.cs`, `SessionLoop.cs` +- `src/SharpMud.Hosting/LoginFlow.cs`, `SharpMudHostOptions.cs`, + `SessionLoop.cs` +- `samples/SharpMud.Samples.Classic/Program.cs` +- `tests/SharpMud.Hosting.Tests/*` (`LoginFlow`/`SharpMudHostOptions` + bootstrap coverage) - `docs/commands.md`, `docs/accounts-auth.md`, `docs/deployment.md`, `SPEC.md`, `docs/adr/README.md`, `docs/plans/README.md`, `docs/plans/0001-wheelmud-reconciliation-roadmap.md` @@ -374,9 +422,9 @@ Modified: distinct power of two, and no two members share a bit (`Enum.GetValues()` pairwise-AND'd should all be `None` except `All`/self-comparisons). `All` equals the OR of every individual - flag. The regression test for the undefined-values gap caught in PR - review — this is the one test that would have caught it immediately if - the enum were ever implemented with auto-numbered members. + flag. This is the one test that would immediately catch the + undefined-values gap if the enum were ever implemented with + auto-numbered members. - Unit: `RoleGuardedCommand` — actor with the required role reaches the inner command; actor without it doesn't, gets the rejection message. Cover the any-of/bitwise semantics (actor has one of several required @@ -394,30 +442,24 @@ Modified: Session is { IsConnected: true }` targets/recipients count. Specifically cover a `Playing`-but-`Session == null` player (the repository-reload case — `ConnectionState` defaults to `Playing` because it isn't - persisted) being correctly treated as *not* online — the regression test - for both the online/live ambiguity caught in self-review and the - reload-defaults-to-Playing gap caught in PR review. + persisted) being correctly treated as *not* online. - Unit: `SessionLoop` — a session ending with `WasBooted` set (but not `explicitQuit`) takes the same immediate-removal path `explicitQuit` - takes, not `EnterLinkdead`. The regression test for the - boot-is-cosmetic gap caught in PR review: without this, a booted player - would just resume via the `Linkdead` reconnect path, making `boot` a - no-op as a moderation tool. + takes, not `EnterLinkdead`. Without this, a booted player would just + resume via the `Linkdead` reconnect path, making `boot` a no-op as a + moderation tool. - Unit: `HelpCommand` — a role-gated command is omitted from the listing for an actor without the required role, and included for one with it; - non-gated commands always list regardless of role. The regression test - for the admin-command-visibility gap caught in self-review. + non-gated commands always list regardless of role. - Unit: `BanCommand` — self-targeting is rejected with a clear message, `IsBanned` unchanged; targeting another player still works normally. - Unit: `RoleRevokeCommand` — revoking your own `FullAdmin` is rejected; revoking a different role from yourself, or `FullAdmin` from someone - else, still works normally. Both are the regression test for the - self-lockout risk caught in self-review. + else, still works normally. - Unit: `RoleGrantCommand`/`RoleRevokeCommand` — `all` and `none` (any casing) are rejected with a clear message and never reach `GrantRole`/`RevokeRole`; every other individually-grantable role name - is accepted. The regression test for the `All`/sentinel-value gap - caught in PR review. + is accepted. - Unit: `LoginFlow` — banned user rejected at password verification with the correct message, not silently falling through. - Unit: `PlayerBehavior.GrantRole`/`RevokeRole`/`Mute`/`Unmute`/`Ban`/ @@ -427,46 +469,41 @@ Modified: `FullBuilder` + `MinorBuilder`. - Unit: a `FullAdmin`-only actor (post-accumulation) successfully passes a `MinorAdmin`-gated `RoleGuardedCommand` — the regression test for the - bootstrap-admin-can't-moderate gap caught in PR review. + bootstrap-admin-can't-moderate gap. - Unit: `RevokeRole(MinorAdmin)` on an actor who also holds `FullAdmin` - returns a non-null failure message and leaves `Roles` unchanged — the - regression test for the revoke-side hierarchy gap caught in PR review - (and, separately, that the failure is a return value, not a thrown - exception, per `coding-standards.md`'s Error Handling section). - `RevokeRole(FullAdmin)` on that same actor returns `null` (success) and - leaves `MinorAdmin`/`Player` intact (demotion, not a full reset — - revoking the top tier doesn't cascade-clear what it implied). - `RevokeRole(MinorAdmin)` on an actor who does *not* also hold - `FullAdmin` returns `null` (success) normally. -- Unit: `HostOptions.Parse` — `SHARPMUD_INITIAL_ADMIN` parses correctly, - absent env var leaves it null. -- Unit: bootstrap grants `FullAdmin` via both paths independently — the - `Program.cs` existing-character path, and `LoginFlow.MaybeCreateAsync`'s - newly-created-character path — since a boot-time-only check was the - fresh-server gap caught in PR review. -- Unit: `LoginFlow.MaybeCreateAsync` (or its `RunAsync` entry point) - actually receives and uses `InitialAdminUsername` — a character created - with a username matching it gets `FullAdmin`; a character created with a - non-matching (or null/absent) `InitialAdminUsername` does not. The - regression test for the parameter-threading gap caught in PR review - (`TelnetHostContext`/`LoginFlow.RunAsync` never carried this value - before). + returns a non-null failure message and leaves `Roles` unchanged (and + that failure is a return value, not a thrown exception, per + `coding-standards.md`'s Error Handling section). `RevokeRole(FullAdmin)` + on that same actor returns `null` (success) and leaves + `MinorAdmin`/`Player` intact (demotion, not a full reset — revoking the + top tier doesn't cascade-clear what it implied). `RevokeRole(MinorAdmin)` + on an actor who does *not* also hold `FullAdmin` returns `null` + (success) normally. +- Unit: `SharpMudHostOptions.Parse` — `SHARPMUD_INITIAL_ADMIN` parses + correctly, absent env var leaves it null. +- Unit: `LoginFlow` — a login (new-character and existing-character paths + both) whose username matches `InitialAdminUsername` gets `FullAdmin` + idempotently (repeat logins don't re-grant or error); a non-matching or + null `InitialAdminUsername` grants nothing. Replaces the original plan's + two-separate-mechanism tests (a Program.cs-level check plus a + `MaybeCreateAsync`-level check) with one set of `LoginFlow`-level tests, + matching the simplified single-checkpoint design. ## Verification Real manual check over Telnet (this repo's established pattern for session/persistence-facing changes): -1. **Fresh-server case** (the gap caught in PR review): boot a brand-new - world with `SHARPMUD_INITIAL_ADMIN=` set, and only *then* - create that character over Telnet — confirm the grant happens at - creation time, not just at boot. Confirm the resulting admin can run - *both* a `FullAdmin`-gated command (e.g. `ban`) *and* a - `MinorAdmin`-gated one (e.g. `boot`) without a separate grant — - validates the accumulation fix. +1. **Fresh-server case**: boot a brand-new world with + `SHARPMUD_INITIAL_ADMIN=` set, and only *then* create that + character over Telnet — confirm the grant happens at creation time. + Confirm the resulting admin can run *both* a `FullAdmin`-gated command + (e.g. `ban`) *and* a `MinorAdmin`-gated one (e.g. `boot`) without a + separate grant — validates the accumulation fix. 2. **Restart case**: restart the server (same world, same `SHARPMUD_INITIAL_ADMIN`); confirm the existing admin's roles are - unaffected (idempotent, no duplicate grants/errors). + unaffected (idempotent, no duplicate grants/errors) after logging back + in. 3. As the admin, `rolegrant minoradmin` on a second character; confirm the second character can now run `boot`/`mute`/ `announce` but not `ban`/`rolegrant`. @@ -478,9 +515,7 @@ session/persistence-facing changes): **immediately reconnect as that player** (within the `Linkdead` grace window) and confirm login goes through the normal username/password prompt from scratch, not a "Welcome back" resume — the regression check - for `WasBooted` actually crossing the `SessionLoop` boundary (PR review - gap: without it, `boot` would be a no-op since the target could just - reconnect and resume where they were). + for `WasBooted` actually crossing the `SessionLoop` boundary. 7. `announce `; confirm every currently-connected session receives it. 8. Confirm a non-admin attempting any of the above gets a clear rejection, From 35e6f36285c552c3281d1c3b135d891739de113a Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 13:44:35 -0400 Subject: [PATCH 02/10] feat: SecurityRole model + RegisterOpen/RegisterWithRole registry split (PLAN-0005) First slice of ADR-0005: the SecurityRole [Flags] enum (explicit power-of-two values, not auto-numbered - see the enum's own doc comment for why that's load-bearing), the RoleGuardedCommand/MuteGuardedCommand Decorator wrappers, and ICommandRegistry's Register(ICommand) replaced with RegisterOpen/RegisterWithRole so every command's access level is a legible, intentional statement at its registration call site. PlayerBehavior gains Roles/IsMuted/IsBanned/WasBooted plus mutation methods (GrantRole/RevokeRole/Mute/Unmute/Ban/Unban/MarkBooted) - GrantRole accumulates implied lower tiers (FullAdmin also grants MinorAdmin+Player), RevokeRole enforces the same invariant symmetrically and returns a failure message rather than throwing when a revoke would break it. Roles/IsMuted/IsBanned are persisted (unlike ConnectionState); WasBooted is transient. Migrated both existing registration call sites (BuiltinCommands, SharpMud.Ruleset.Rpg's ServiceCollectionExtensions) to RegisterOpen, and wrapped say/emote specifically in MuteGuardedCommand at the Engine level so mute enforcement is universal, not per-ruleset. HelpCommand now filters role-gated commands out of its listing for actors who can't run them - RoleGuardedCommand passes Verb/Aliases straight through from the wrapped command, so without this every admin command would list unconditionally to every player. Co-Authored-By: Claude Sonnet 5 --- ...rity-role-model-and-moderation-commands.md | 2 +- .../Behaviors/PlayerBehavior.cs | 92 +++++++++++++ .../Commands/Builtin/BuiltinCommands.cs | 56 ++++---- .../Commands/Builtin/HelpCommand.cs | 12 ++ .../Commands/CommandRegistry.cs | 19 ++- .../Commands/ICommandRegistry.cs | 17 ++- .../Commands/MuteGuardedCommand.cs | 41 ++++++ .../Commands/RoleGuardedCommand.cs | 49 +++++++ src/SharpMud.Engine/Commands/SecurityRole.cs | 66 +++++++++ .../Commands/SecurityRoleExtensions.cs | 33 +++++ .../PlayerBehaviorConfiguration.cs | 13 ++ .../ServiceCollectionExtensions.cs | 4 +- .../Behaviors/PlayerBehaviorSecurityTests.cs | 125 ++++++++++++++++++ .../Commands/CommandRegistryTests.cs | 32 ++++- .../Commands/HelpCommandTests.cs | 76 +++++++++++ .../Commands/MuteGuardedCommandTests.cs | 61 +++++++++ .../Commands/RoleGuardedCommandTests.cs | 83 ++++++++++++ .../Commands/SecurityRoleTests.cs | 85 ++++++++++++ .../ServiceCollectionExtensionsTests.cs | 2 +- 19 files changed, 827 insertions(+), 41 deletions(-) create mode 100644 src/SharpMud.Engine/Commands/MuteGuardedCommand.cs create mode 100644 src/SharpMud.Engine/Commands/RoleGuardedCommand.cs create mode 100644 src/SharpMud.Engine/Commands/SecurityRole.cs create mode 100644 src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs create mode 100644 tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/MuteGuardedCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/RoleGuardedCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs diff --git a/docs/plans/0005-security-role-model-and-moderation-commands.md b/docs/plans/0005-security-role-model-and-moderation-commands.md index 252c6b2..43b0850 100644 --- a/docs/plans/0005-security-role-model-and-moderation-commands.md +++ b/docs/plans/0005-security-role-model-and-moderation-commands.md @@ -2,7 +2,7 @@ **Implements:** [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) -**Status:** Not Started +**Status:** In Progress **Last updated:** 2026-07-23 diff --git a/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs b/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs index 8b56d8f..3e97e56 100644 --- a/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs +++ b/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs @@ -1,3 +1,4 @@ +using SharpMud.Engine.Commands; using SharpMud.Engine.Core; using SharpMud.Engine.Sessions; @@ -29,6 +30,37 @@ public sealed class PlayerBehavior : Behavior /// public DateTimeOffset? LinkdeadSinceUtc { get; private set; } + /// + /// This player's granted access levels - persisted (unlike , a role assignment must survive a restart). + /// Defaults to for new characters. + /// Only mutated via / - + /// ADR-0005. + /// + public SecurityRole Roles { get; private set; } = SecurityRole.Player; + + /// Whether this player's say/emote are currently blocked - persisted. Only mutated via /. + public bool IsMuted { get; private set; } + + /// Whether this player is blocked from logging in - persisted, enforced in LoginFlow. Only mutated via /. + public bool IsBanned { get; private set; } + + /// + /// Whether this player's session was just forcibly disconnected by + /// BootCommand (or BanCommand disconnecting an online + /// target). Transient, like / (Ignore'd in + /// PlayerBehaviorConfiguration) - it only needs to survive long + /// enough for this same connection's SessionLoop.RunAsync to see + /// it in its own finally block, crossing from the admin's call + /// stack to the target's. Without this, an admin-triggered disconnect + /// looks identical to a dropped connection, so the target would just + /// resume via the normal + /// reconnect path (ADR-0004) - making boot a no-op as a + /// moderation tool. Only set via . + /// + public bool WasBooted { get; private set; } + /// /// Transitions this player to - called by /// SessionLoop when a connection is lost (not an explicit quit). @@ -57,4 +89,64 @@ public void Reconnect() ConnectionState = ConnectionState.Playing; LinkdeadSinceUtc = null; } + + /// + /// Marks this player as having just been forcibly disconnected - called + /// by BootCommand/BanCommand before disconnecting the + /// target's session. See . + /// + public void MarkBooted() => WasBooted = true; + + /// + /// Grants and every role it implies (e.g. + /// granting also grants and ) + /// - ADR-0005's accumulation rule. Idempotent. + /// + public void GrantRole(SecurityRole role) => Roles |= role.ImpliedRoles; + + /// + /// Revokes , unless some other role this player + /// currently holds implies it (e.g. revoking while still holding would otherwise leave set with a role it implies cleared). + /// Returns on success, or a message naming the + /// blocking higher tier on failure - a normal, directly + /// user-triggerable business-rule outcome per + /// coding-standards.md's Error Handling section, not a thrown + /// exception. + /// + public string? RevokeRole(SecurityRole role) + { + foreach (var candidate in RolesWithImplications) + { + if (candidate == role) + continue; + + if ((Roles & candidate) == candidate && candidate.Implies(role)) + return $"Still has {candidate}, which includes {role} - revoke {candidate} instead."; + } + + Roles &= ~role; + return null; + } + + /// Blocks this player's say/emote. Idempotent. + public void Mute() => IsMuted = true; + + /// Restores this player's say/emote. Idempotent. + public void Unmute() => IsMuted = false; + + /// Blocks this player from logging in. Idempotent. + public void Ban() => IsBanned = true; + + /// Restores this player's ability to log in. Idempotent. + public void Unban() => IsBanned = false; + + // The only roles with an "implies" relationship to another role - see + // SecurityRoleExtensions.ImpliedRoles. RevokeRole only needs to check + // these three, not every SecurityRole member. + private static readonly SecurityRole[] RolesWithImplications = + [SecurityRole.FullAdmin, SecurityRole.MinorAdmin, SecurityRole.FullBuilder]; } diff --git a/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs b/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs index 7130feb..bf909cd 100644 --- a/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs +++ b/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs @@ -4,34 +4,42 @@ namespace SharpMud.Engine.Commands.Builtin; // Ruleset-agnostic commands only - kill/attack/flee (and anything else that // depends on a ruleset-specific behavior) register themselves separately; -// see SharpMud.Samples.Classic's equivalent registration method, called by -// Host alongside this one. +// see SharpMud.Ruleset.Rpg's equivalent registration, called by Hosting's +// AddSharpMudRuleset alongside this one. Admin/moderation commands +// (ADR-0005) also register separately (SharpMud.Engine.Commands.Builtin +// .Admin.AdminCommands) - they're role-gated, not ruleset-specific, but +// keeping them out of this method keeps "the commands every consumer gets +// with zero configuration" and "the commands a consumer opts into wiring up +// themselves" visibly separate. public static class BuiltinCommands { public static void RegisterAll(ICommandRegistry registry) { - registry.Register(new MoveCommand(Direction.North, "north", ["n"])); - registry.Register(new MoveCommand(Direction.South, "south", ["s"])); - registry.Register(new MoveCommand(Direction.East, "east", ["e"])); - registry.Register(new MoveCommand(Direction.West, "west", ["w"])); - registry.Register(new MoveCommand(Direction.NorthEast, "northeast", ["ne"])); - registry.Register(new MoveCommand(Direction.NorthWest, "northwest", ["nw"])); - registry.Register(new MoveCommand(Direction.SouthEast, "southeast", ["se"])); - registry.Register(new MoveCommand(Direction.SouthWest, "southwest", ["sw"])); - registry.Register(new MoveCommand(Direction.Up, "up", ["u"])); - registry.Register(new MoveCommand(Direction.Down, "down", ["d"])); + registry.RegisterOpen(new MoveCommand(Direction.North, "north", ["n"])); + registry.RegisterOpen(new MoveCommand(Direction.South, "south", ["s"])); + registry.RegisterOpen(new MoveCommand(Direction.East, "east", ["e"])); + registry.RegisterOpen(new MoveCommand(Direction.West, "west", ["w"])); + registry.RegisterOpen(new MoveCommand(Direction.NorthEast, "northeast", ["ne"])); + registry.RegisterOpen(new MoveCommand(Direction.NorthWest, "northwest", ["nw"])); + registry.RegisterOpen(new MoveCommand(Direction.SouthEast, "southeast", ["se"])); + registry.RegisterOpen(new MoveCommand(Direction.SouthWest, "southwest", ["sw"])); + registry.RegisterOpen(new MoveCommand(Direction.Up, "up", ["u"])); + registry.RegisterOpen(new MoveCommand(Direction.Down, "down", ["d"])); - registry.Register(new LookCommand()); - registry.Register(new SayCommand()); - registry.Register(new EmoteCommand()); - registry.Register(new WhoCommand()); - registry.Register(new QuitCommand()); - registry.Register(new GetCommand()); - registry.Register(new DropCommand()); - registry.Register(new WearCommand()); - registry.Register(new RemoveCommand()); - registry.Register(new InventoryCommand()); - registry.Register(new GiveCommand()); - registry.Register(new HelpCommand(registry)); + registry.RegisterOpen(new LookCommand()); + // Mute enforcement (ADR-0005) is an Engine-level concern, not + // per-consumer - wrapping here, not in each ruleset's own + // registration, is what makes mute universal. + registry.RegisterOpen(new MuteGuardedCommand(new SayCommand())); + registry.RegisterOpen(new MuteGuardedCommand(new EmoteCommand())); + registry.RegisterOpen(new WhoCommand()); + registry.RegisterOpen(new QuitCommand()); + registry.RegisterOpen(new GetCommand()); + registry.RegisterOpen(new DropCommand()); + registry.RegisterOpen(new WearCommand()); + registry.RegisterOpen(new RemoveCommand()); + registry.RegisterOpen(new InventoryCommand()); + registry.RegisterOpen(new GiveCommand()); + registry.RegisterOpen(new HelpCommand(registry)); } } diff --git a/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs b/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs index 1336730..a2a0785 100644 --- a/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs @@ -1,3 +1,5 @@ +using SharpMud.Engine.Behaviors; + namespace SharpMud.Engine.Commands.Builtin; public sealed class HelpCommand(ICommandRegistry registry) : ICommand @@ -8,8 +10,18 @@ public sealed class HelpCommand(ICommandRegistry registry) : ICommand public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) { await ctx.Session.WriteLineAsync("Available commands:", ct); + + var actorRoles = ctx.Actor.FindBehavior()?.Roles ?? SecurityRole.None; foreach (var command in registry.Commands.OrderBy(c => c.Verb, StringComparer.Ordinal)) { + // RoleGuardedCommand passes Verb/Aliases straight through from + // the command it wraps, so without this check every admin + // command would list unconditionally to every player - not an + // exploit (the gate still blocks execution) but a real, + // unpolished info leak. + if (command is RoleGuardedCommand guarded && (actorRoles & guarded.RequiredRole) == SecurityRole.None) + continue; + var aliasSuffix = command.Aliases.Count > 0 ? $" ({string.Join(", ", command.Aliases)})" : ""; diff --git a/src/SharpMud.Engine/Commands/CommandRegistry.cs b/src/SharpMud.Engine/Commands/CommandRegistry.cs index adab9c8..2019b12 100644 --- a/src/SharpMud.Engine/Commands/CommandRegistry.cs +++ b/src/SharpMud.Engine/Commands/CommandRegistry.cs @@ -13,14 +13,10 @@ public sealed class CommandRegistry : ICommandRegistry public IReadOnlyList Commands => _commands; - public void Register(ICommand command) - { - _verbs[command.Verb] = command; - foreach (var alias in command.Aliases) - _aliases.TryAdd(alias, command); + public void RegisterOpen(ICommand command) => RegisterInternal(command); - _commands.Add(command); - } + public void RegisterWithRole(ICommand command, SecurityRole requiredRole) => + RegisterInternal(new RoleGuardedCommand(command, requiredRole)); public bool TryResolve(string verb, [MaybeNullWhen(false)] out ICommand command) { @@ -29,4 +25,13 @@ public bool TryResolve(string verb, [MaybeNullWhen(false)] out ICommand command) return _aliases.TryGetValue(verb, out command); } + + private void RegisterInternal(ICommand command) + { + _verbs[command.Verb] = command; + foreach (var alias in command.Aliases) + _aliases.TryAdd(alias, command); + + _commands.Add(command); + } } diff --git a/src/SharpMud.Engine/Commands/ICommandRegistry.cs b/src/SharpMud.Engine/Commands/ICommandRegistry.cs index 3365d38..0bd35f9 100644 --- a/src/SharpMud.Engine/Commands/ICommandRegistry.cs +++ b/src/SharpMud.Engine/Commands/ICommandRegistry.cs @@ -2,10 +2,25 @@ namespace SharpMud.Engine.Commands; +/// +/// Resolves verbs/aliases to registered commands. Registration has exactly +/// two intentional entry points, and - there is no plain Register(ICommand), +/// per ADR-0005: every command's access level must be a legible, +/// intentional statement at its registration call site, not an easily +/// forgotten guard call inside ExecuteAsync. +/// public interface ICommandRegistry { + /// Every registered command, each counted once regardless of alias count. IReadOnlyList Commands { get; } - void Register(ICommand command); + /// Registers a command with no access restriction - anyone can run it. + void RegisterOpen(ICommand command); + + /// Registers a command wrapped in a requiring at least one of 's flags. + void RegisterWithRole(ICommand command, SecurityRole requiredRole); + + /// Attempts to resolve a verb or alias to its registered command. bool TryResolve(string verb, [MaybeNullWhen(false)] out ICommand command); } diff --git a/src/SharpMud.Engine/Commands/MuteGuardedCommand.cs b/src/SharpMud.Engine/Commands/MuteGuardedCommand.cs new file mode 100644 index 0000000..faabebe --- /dev/null +++ b/src/SharpMud.Engine/Commands/MuteGuardedCommand.cs @@ -0,0 +1,41 @@ +using SharpMud.Engine.Behaviors; + +namespace SharpMud.Engine.Commands; + +/// +/// Wraps an inner so it only executes for an actor +/// whose own PlayerBehavior.IsMuted is - the +/// same Decorator shape as , reused for a +/// cross-cutting concern that isn't role-based at all (ADR-0005). Gates the +/// *actor's own* ability to speak (say/emote), not anything +/// they're doing to a target. +/// +public sealed class MuteGuardedCommand : ICommand +{ + private readonly ICommand _inner; + + /// Wraps . + public MuteGuardedCommand(ICommand inner) + { + _inner = inner; + } + + /// + public string Verb => _inner.Verb; + + /// + public IReadOnlyList Aliases => _inner.Aliases; + + /// Checks the actor's own mute state, then delegates to the wrapped command or sends a rejection message. + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + var isMuted = ctx.Actor.FindBehavior()?.IsMuted ?? false; + if (isMuted) + { + await ctx.Session.WriteLineAsync("You have been muted and cannot do that.", ct); + return; + } + + await _inner.ExecuteAsync(ctx, ct); + } +} diff --git a/src/SharpMud.Engine/Commands/RoleGuardedCommand.cs b/src/SharpMud.Engine/Commands/RoleGuardedCommand.cs new file mode 100644 index 0000000..4674de7 --- /dev/null +++ b/src/SharpMud.Engine/Commands/RoleGuardedCommand.cs @@ -0,0 +1,49 @@ +using SharpMud.Engine.Behaviors; + +namespace SharpMud.Engine.Commands; + +/// +/// Wraps an inner so it only executes for an actor +/// holding at least one of 's flags (any-of +/// semantics, bitwise AND) - the Decorator mechanism ADR-0005 chose over a +/// forced ICommand.RequiredRole interface member. Created by , not meant to be constructed +/// directly by ordinary command registration code. +/// +public sealed class RoleGuardedCommand : ICommand +{ + private readonly ICommand _inner; + + /// Wraps , requiring at least one of 's flags. + public RoleGuardedCommand(ICommand inner, SecurityRole requiredRole) + { + _inner = inner; + RequiredRole = requiredRole; + } + + /// + /// The role(s) an actor must hold at least one of to reach the wrapped + /// command - exposed publicly so HelpCommand can filter its + /// listing by the same check, not just internally. + /// + public SecurityRole RequiredRole { get; } + + /// + public string Verb => _inner.Verb; + + /// + public IReadOnlyList Aliases => _inner.Aliases; + + /// Checks the actor's roles, then delegates to the wrapped command or sends a rejection message. + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + var actorRoles = ctx.Actor.FindBehavior()?.Roles ?? SecurityRole.None; + if ((actorRoles & RequiredRole) == SecurityRole.None) + { + await ctx.Session.WriteLineAsync("You don't have permission to do that.", ct); + return; + } + + await _inner.ExecuteAsync(ctx, ct); + } +} diff --git a/src/SharpMud.Engine/Commands/SecurityRole.cs b/src/SharpMud.Engine/Commands/SecurityRole.cs new file mode 100644 index 0000000..3eb12fc --- /dev/null +++ b/src/SharpMud.Engine/Commands/SecurityRole.cs @@ -0,0 +1,66 @@ +namespace SharpMud.Engine.Commands; + +/// +/// Access-level flags checked by (bitwise +/// AND, any-of semantics) - ported in full from WheelMUD's own +/// SecurityRole (see docs/research/wheelmud-findings.md §11), even +/// though several values (////) have no +/// consumer in sharp-mud yet - see ADR-0005's Decision Outcome for why. +/// +/// +/// Explicit power-of-two values on every member are load-bearing, not +/// stylistic - C#'s default sequential auto-numbering (0, 1, 2, 3...) +/// would not produce distinct bits (e.g. would silently +/// equal | combined), breaking +/// 's bitwise-AND check into granting +/// unrelated permissions on overlapping bits. is defined +/// as the union of every individual flag, not a separate hardcoded value, +/// so it can't drift out of sync if a flag is added later. +/// +[Flags] +public enum SecurityRole : uint +{ + /// No roles. + None = 0, + + /// A non-player command issuer (e.g. an NPC). No consumer yet. + Mobile = 1 << 0, + + /// An item-driven command issuer. No consumer yet. + Item = 1 << 1, + + /// A room-driven command issuer. No consumer yet. + Room = 1 << 2, + + /// A player still in a tutorial/onboarding flow. No consumer yet. + TutorialPlayer = 1 << 3, + + /// An ordinary player - the default role every new character starts with. + Player = 1 << 4, + + /// A player granted helper-tier privileges. No consumer yet. + Helper = 1 << 5, + + /// A player in a married-couple relationship. No consumer yet (no marriage system). + Married = 1 << 6, + + /// Limited world-building access (Slice 4). + MinorBuilder = 1 << 7, + + /// Full world-building access (Slice 4). Implies . + FullBuilder = 1 << 8, + + /// Day-to-day moderation: boot/mute/unmute/announce. + MinorAdmin = 1 << 9, + + /// + /// Full administration: everything can do, plus + /// ban/unban/rolegrant/rolerevoke. Implies + /// and . + /// + FullAdmin = 1 << 10, + + /// The union of every individual role above. + All = Mobile | Item | Room | TutorialPlayer | Player | Helper | Married | MinorBuilder | FullBuilder | MinorAdmin | FullAdmin, +} diff --git a/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs b/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs new file mode 100644 index 0000000..c73a316 --- /dev/null +++ b/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs @@ -0,0 +1,33 @@ +namespace SharpMud.Engine.Commands; + +/// +/// The role hierarchy ( implies implies ; +/// implies ), defined once here and used by both +/// PlayerBehavior.GrantRole (accumulate downward) and +/// PlayerBehavior.RevokeRole (check upward) per ADR-0005's +/// accumulation rule. +/// +public static class SecurityRoleExtensions +{ + extension(SecurityRole role) + { + /// + /// itself, plus every role it implies. + /// Assumes is a single flag - this repo's + /// only callers (GrantRole, admin commands validated + /// against an individually-grantable allowlist) always pass one. + /// + public SecurityRole ImpliedRoles => role switch + { + SecurityRole.FullAdmin => SecurityRole.FullAdmin | SecurityRole.MinorAdmin | SecurityRole.Player, + SecurityRole.MinorAdmin => SecurityRole.MinorAdmin | SecurityRole.Player, + SecurityRole.FullBuilder => SecurityRole.FullBuilder | SecurityRole.MinorBuilder, + _ => role, + }; + + /// Whether holding implies . + public bool Implies(SecurityRole other) => (role.ImpliedRoles & other) == other; + } +} diff --git a/src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs b/src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs index 20ccbce..48d4451 100644 --- a/src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs +++ b/src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs @@ -24,5 +24,18 @@ public void Configure(EntityTypeBuilder builder) // (ADR-0004). builder.Ignore(x => x.ConnectionState); builder.Ignore(x => x.LinkdeadSinceUtc); + + // Roles/IsMuted/IsBanned (ADR-0005) - unlike ConnectionState, these + // must survive a restart or they're useless. Roles is a plain + // [Flags] enum - default int mapping, same as WearableBehavior.Slot, + // no custom value converter needed. + builder.Property(x => x.Roles); + builder.Property(x => x.IsMuted); + builder.Property(x => x.IsBanned); + + // WasBooted is transient, same category as Session/ConnectionState + // above - it only needs to survive within one already-live process, + // never across a restart (see PlayerBehavior.WasBooted's doc comment). + builder.Ignore(x => x.WasBooted); } } diff --git a/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs b/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs index aa5146d..1de2eaf 100644 --- a/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs +++ b/src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs @@ -50,8 +50,8 @@ public static IServiceCollection AddSharpMudRpgRuleset( services.AddSharpMudRuleset((sp, registry) => { - registry.Register(new AttackCommand(sp.GetRequiredService())); - registry.Register(new FleeCommand( + registry.RegisterOpen(new AttackCommand(sp.GetRequiredService())); + registry.RegisterOpen(new FleeCommand( sp.GetRequiredService(), sp.GetRequiredService(), sp.GetRequiredService())); diff --git a/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs b/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs new file mode 100644 index 0000000..707a4cd --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs @@ -0,0 +1,125 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; + +namespace SharpMud.Engine.Tests.Behaviors; + +public sealed class PlayerBehaviorSecurityTests +{ + private static PlayerBehavior MakePlayer() => new() { Username = "TestUser", PasswordHash = "test-hash" }; + + [Fact] + public void Roles_DefaultsToPlayer_ForANewCharacter() + { + var player = MakePlayer(); + + player.Roles.Should().Be(SecurityRole.Player); + } + + [Fact] + public void GrantRole_FullAdmin_AlsoGrantsMinorAdminAndPlayer() + { + var player = MakePlayer(); + + player.GrantRole(SecurityRole.FullAdmin); + + player.Roles.Should().HaveFlag(SecurityRole.FullAdmin); + player.Roles.Should().HaveFlag(SecurityRole.MinorAdmin); + player.Roles.Should().HaveFlag(SecurityRole.Player); + } + + [Fact] + public void GrantRole_FullBuilder_AlsoGrantsMinorBuilder() + { + var player = MakePlayer(); + + player.GrantRole(SecurityRole.FullBuilder); + + player.Roles.Should().HaveFlag(SecurityRole.FullBuilder); + player.Roles.Should().HaveFlag(SecurityRole.MinorBuilder); + } + + [Fact] + public void GrantRole_IsIdempotent() + { + var player = MakePlayer(); + player.GrantRole(SecurityRole.FullAdmin); + + var act = () => player.GrantRole(SecurityRole.FullAdmin); + + act.Should().NotThrow(); + player.Roles.Should().HaveFlag(SecurityRole.FullAdmin); + } + + [Fact] + public void RevokeRole_MinorAdmin_ReturnsFailureAndLeavesRolesUnchanged_WhenActorAlsoHoldsFullAdmin() + { + var player = MakePlayer(); + player.GrantRole(SecurityRole.FullAdmin); + var rolesBefore = player.Roles; + + var result = player.RevokeRole(SecurityRole.MinorAdmin); + + result.Should().NotBeNull(); + player.Roles.Should().Be(rolesBefore); + } + + [Fact] + public void RevokeRole_FullAdmin_SucceedsAndLeavesMinorAdminAndPlayerIntact_WhenActorHoldsFullAdmin() + { + var player = MakePlayer(); + player.GrantRole(SecurityRole.FullAdmin); + + var result = player.RevokeRole(SecurityRole.FullAdmin); + + result.Should().BeNull(); + player.Roles.Should().NotHaveFlag(SecurityRole.FullAdmin); + player.Roles.Should().HaveFlag(SecurityRole.MinorAdmin); + player.Roles.Should().HaveFlag(SecurityRole.Player); + } + + [Fact] + public void RevokeRole_MinorAdmin_Succeeds_WhenActorDoesNotAlsoHoldFullAdmin() + { + var player = MakePlayer(); + player.GrantRole(SecurityRole.MinorAdmin); + + var result = player.RevokeRole(SecurityRole.MinorAdmin); + + result.Should().BeNull(); + player.Roles.Should().NotHaveFlag(SecurityRole.MinorAdmin); + } + + [Fact] + public void Mute_SetsIsMuted_AndUnmute_ClearsIt() + { + var player = MakePlayer(); + + player.Mute(); + player.IsMuted.Should().BeTrue(); + + player.Unmute(); + player.IsMuted.Should().BeFalse(); + } + + [Fact] + public void Ban_SetsIsBanned_AndUnban_ClearsIt() + { + var player = MakePlayer(); + + player.Ban(); + player.IsBanned.Should().BeTrue(); + + player.Unban(); + player.IsBanned.Should().BeFalse(); + } + + [Fact] + public void MarkBooted_SetsWasBooted() + { + var player = MakePlayer(); + + player.MarkBooted(); + + player.WasBooted.Should().BeTrue(); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/CommandRegistryTests.cs b/tests/SharpMud.Engine.Tests/Commands/CommandRegistryTests.cs index ed3cb92..7d1cdc8 100644 --- a/tests/SharpMud.Engine.Tests/Commands/CommandRegistryTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/CommandRegistryTests.cs @@ -17,7 +17,7 @@ private sealed class FakeCommand(string verb, params string[] aliases) : IComman public void TryResolve_ReturnsCommand_WhenVerbMatchesCanonicalVerb() { var north = new FakeCommand("north", "n"); - _sut.Register(north); + _sut.RegisterOpen(north); _sut.TryResolve("north", out var resolved).Should().BeTrue(); resolved.Should().BeSameAs(north); @@ -27,7 +27,7 @@ public void TryResolve_ReturnsCommand_WhenVerbMatchesCanonicalVerb() public void TryResolve_ReturnsCommand_WhenVerbMatchesAlias() { var north = new FakeCommand("north", "n"); - _sut.Register(north); + _sut.RegisterOpen(north); _sut.TryResolve("n", out var resolved).Should().BeTrue(); resolved.Should().BeSameAs(north); @@ -50,8 +50,8 @@ public void TryResolve_PrefersCanonicalVerb_WhenAliasCollidesWithAnotherCommands var north = new FakeCommand("north", "n"); var literalN = new FakeCommand("n"); - _sut.Register(north); - _sut.Register(literalN); + _sut.RegisterOpen(north); + _sut.RegisterOpen(literalN); _sut.TryResolve("n", out var resolved).Should().BeTrue(); resolved.Should().BeSameAs(literalN); @@ -61,8 +61,30 @@ public void TryResolve_PrefersCanonicalVerb_WhenAliasCollidesWithAnotherCommands public void Commands_ContainsEveryRegisteredCommandOnce_RegardlessOfAliasCount() { var north = new FakeCommand("north", "n"); - _sut.Register(north); + _sut.RegisterOpen(north); _sut.Commands.Should().ContainSingle().Which.Should().BeSameAs(north); } + + [Fact] + public void RegisterOpen_ResolvesUnconditionally() + { + var command = new FakeCommand("dance"); + _sut.RegisterOpen(command); + + _sut.TryResolve("dance", out var resolved).Should().BeTrue(); + resolved.Should().BeSameAs(command); + } + + [Fact] + public void RegisterWithRole_ResolvesToARoleGuardedCommandWrappingTheGivenCommandAndRole() + { + var command = new FakeCommand("ban"); + _sut.RegisterWithRole(command, SecurityRole.FullAdmin); + + _sut.TryResolve("ban", out var resolved).Should().BeTrue(); + var guarded = resolved.Should().BeOfType().Subject; + guarded.RequiredRole.Should().Be(SecurityRole.FullAdmin); + guarded.Verb.Should().Be("ban"); + } } diff --git a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs new file mode 100644 index 0000000..f40800c --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs @@ -0,0 +1,76 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands; + +public sealed class HelpCommandTests +{ + private sealed class FakeCommand(string verb) : ICommand + { + public string Verb { get; } = verb; + public IReadOnlyList Aliases { get; } = []; + public Task ExecuteAsync(CommandContext ctx, CancellationToken ct) => Task.CompletedTask; + } + + private static Thing MakeActor(SecurityRole roles) + { + var actor = new Thing { Id = ThingId.New(), Name = "Actor" }; + var behavior = new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash" }; + behavior.GrantRole(roles); + actor.Behaviors.Add(behavior); + return actor; + } + + [Fact] + public async Task ExecuteAsync_OmitsRoleGatedCommand_WhenActorLacksTheRequiredRole() + { + var session = Substitute.For(); + var registry = new CommandRegistry(); + registry.RegisterOpen(new FakeCommand("look")); + registry.RegisterWithRole(new FakeCommand("ban"), SecurityRole.FullAdmin); + + var actor = MakeActor(SecurityRole.Player); + var sut = new HelpCommand(registry); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.DidNotReceive().WriteLineAsync(Arg.Is(s => s.Contains("ban")), Arg.Any()); + await session.Received(1).WriteLineAsync(Arg.Is(s => s.Contains("look")), Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_IncludesRoleGatedCommand_WhenActorHasTheRequiredRole() + { + var session = Substitute.For(); + var registry = new CommandRegistry(); + registry.RegisterWithRole(new FakeCommand("ban"), SecurityRole.FullAdmin); + + var actor = MakeActor(SecurityRole.FullAdmin); + var sut = new HelpCommand(registry); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync(Arg.Is(s => s.Contains("ban")), Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_AlwaysIncludesNonGatedCommands_RegardlessOfRole() + { + var session = Substitute.For(); + var registry = new CommandRegistry(); + registry.RegisterOpen(new FakeCommand("look")); + + var actor = MakeActor(SecurityRole.Player); + var sut = new HelpCommand(registry); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync(Arg.Is(s => s.Contains("look")), Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/MuteGuardedCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/MuteGuardedCommandTests.cs new file mode 100644 index 0000000..b35e063 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/MuteGuardedCommandTests.cs @@ -0,0 +1,61 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands; + +public sealed class MuteGuardedCommandTests +{ + private sealed class FakeCommand : ICommand + { + public bool WasExecuted { get; private set; } + public string Verb => "say"; + public IReadOnlyList Aliases { get; } = []; + + public Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + WasExecuted = true; + return Task.CompletedTask; + } + } + + private static Thing MakeActor(bool muted) + { + var actor = new Thing { Id = ThingId.New(), Name = "Actor" }; + var behavior = new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash" }; + if (muted) + behavior.Mute(); + actor.Behaviors.Add(behavior); + return actor; + } + + [Fact] + public async Task ExecuteAsync_DelegatesToInnerCommand_WhenActorIsNotMuted() + { + var session = Substitute.For(); + var actor = MakeActor(muted: false); + var inner = new FakeCommand(); + var sut = new MuteGuardedCommand(inner); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + inner.WasExecuted.Should().BeTrue(); + } + + [Fact] + public async Task ExecuteAsync_SendsRejectionMessage_WhenActorIsMuted() + { + var session = Substitute.For(); + var actor = MakeActor(muted: true); + var inner = new FakeCommand(); + var sut = new MuteGuardedCommand(inner); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + inner.WasExecuted.Should().BeFalse(); + await session.Received(1).WriteLineAsync("You have been muted and cannot do that.", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/RoleGuardedCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/RoleGuardedCommandTests.cs new file mode 100644 index 0000000..ff13bb1 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/RoleGuardedCommandTests.cs @@ -0,0 +1,83 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands; + +public sealed class RoleGuardedCommandTests +{ + private sealed class FakeCommand : ICommand + { + public bool WasExecuted { get; private set; } + public string Verb => "ban"; + public IReadOnlyList Aliases { get; } = ["b"]; + + public Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + WasExecuted = true; + return Task.CompletedTask; + } + } + + private static Thing MakeActor(SecurityRole roles) + { + var actor = new Thing { Id = ThingId.New(), Name = "Actor" }; + var behavior = new PlayerBehavior { Username = "TestUser", PasswordHash = "test-hash" }; + behavior.GrantRole(roles); + actor.Behaviors.Add(behavior); + return actor; + } + + [Fact] + public void VerbAndAliases_PassThroughFromTheWrappedCommand() + { + var sut = new RoleGuardedCommand(new FakeCommand(), SecurityRole.FullAdmin); + + sut.Verb.Should().Be("ban"); + sut.Aliases.Should().Equal("b"); + } + + [Fact] + public async Task ExecuteAsync_DelegatesToInnerCommand_WhenActorHasTheRequiredRole() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.FullAdmin); + var inner = new FakeCommand(); + var sut = new RoleGuardedCommand(inner, SecurityRole.FullAdmin); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + inner.WasExecuted.Should().BeTrue(); + } + + [Fact] + public async Task ExecuteAsync_SendsRejectionMessage_WhenActorLacksTheRequiredRole() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.Player); + var inner = new FakeCommand(); + var sut = new RoleGuardedCommand(inner, SecurityRole.FullAdmin); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + inner.WasExecuted.Should().BeFalse(); + await session.Received(1).WriteLineAsync("You don't have permission to do that.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_Delegates_WhenActorHasAtLeastOneOfSeveralRequiredFlags() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.MinorAdmin); + var inner = new FakeCommand(); + var sut = new RoleGuardedCommand(inner, SecurityRole.MinorAdmin | SecurityRole.FullAdmin); + var ctx = new CommandContext(actor, actor, [], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + inner.WasExecuted.Should().BeTrue(); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs b/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs new file mode 100644 index 0000000..ebd86ef --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs @@ -0,0 +1,85 @@ +using SharpMud.Engine.Commands; + +namespace SharpMud.Engine.Tests.Commands; + +public sealed class SecurityRoleTests +{ + private static readonly SecurityRole[] IndividualRoles = + [ + SecurityRole.Mobile, + SecurityRole.Item, + SecurityRole.Room, + SecurityRole.TutorialPlayer, + SecurityRole.Player, + SecurityRole.Helper, + SecurityRole.Married, + SecurityRole.MinorBuilder, + SecurityRole.FullBuilder, + SecurityRole.MinorAdmin, + SecurityRole.FullAdmin, + ]; + + // The regression test for the undefined-values gap: if SecurityRole + // were ever implemented with auto-numbered members instead of explicit + // power-of-two values, at least one pair here would share a bit and + // this test would catch it immediately. + [Fact] + public void EveryIndividualRole_IsADistinctPowerOfTwo() + { + foreach (var role in IndividualRoles) + { + var value = (uint)role; + (value != 0 && (value & (value - 1)) == 0).Should().BeTrue($"{role} must be a single bit"); + } + + for (var i = 0; i < IndividualRoles.Length; i++) + { + for (var j = i + 1; j < IndividualRoles.Length; j++) + (IndividualRoles[i] & IndividualRoles[j]).Should().Be(SecurityRole.None, $"{IndividualRoles[i]} and {IndividualRoles[j]} must not share a bit"); + } + } + + [Fact] + public void All_EqualsTheUnionOfEveryIndividualRole() + { + var union = IndividualRoles.Aggregate(SecurityRole.None, (acc, role) => acc | role); + + SecurityRole.All.Should().Be(union); + } + + [Fact] + public void ImpliedRoles_ForFullAdmin_IncludesMinorAdminAndPlayer() + { + var implied = SecurityRole.FullAdmin.ImpliedRoles; + + implied.Should().HaveFlag(SecurityRole.FullAdmin); + implied.Should().HaveFlag(SecurityRole.MinorAdmin); + implied.Should().HaveFlag(SecurityRole.Player); + } + + [Fact] + public void ImpliedRoles_ForFullBuilder_IncludesMinorBuilder() + { + var implied = SecurityRole.FullBuilder.ImpliedRoles; + + implied.Should().HaveFlag(SecurityRole.FullBuilder); + implied.Should().HaveFlag(SecurityRole.MinorBuilder); + } + + [Fact] + public void ImpliedRoles_ForARoleWithNoHierarchy_IsJustItself() + { + SecurityRole.Helper.ImpliedRoles.Should().Be(SecurityRole.Helper); + } + + [Theory] + [InlineData(SecurityRole.FullAdmin, SecurityRole.MinorAdmin, true)] + [InlineData(SecurityRole.FullAdmin, SecurityRole.Player, true)] + [InlineData(SecurityRole.MinorAdmin, SecurityRole.FullAdmin, false)] + [InlineData(SecurityRole.FullBuilder, SecurityRole.MinorBuilder, true)] + [InlineData(SecurityRole.MinorBuilder, SecurityRole.FullBuilder, false)] + public void Implies_ReflectsTheRoleHierarchy(SecurityRole role, SecurityRole other, bool expected) + { + role.Implies(other).Should().Be(expected); + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs index 3f8cc35..a9d2ad3 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs @@ -19,7 +19,7 @@ public void AddSharpMudRpgRuleset_ComposesBuiltinRpgAndConsumerCommands_IntoOneR services.AddSingleton(Substitute.For()); services.AddSharpMudRpgRuleset((_, registry) => - registry.Register(new FakeConsumerCommand())); + registry.RegisterOpen(new FakeConsumerCommand())); var provider = services.BuildServiceProvider(); var registry = provider.GetRequiredService(); From 0d9afd8410391cfd37d6f064d4cc9d3bce552bdf Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 13:51:29 -0400 Subject: [PATCH 03/10] feat: 8 moderation commands + AdminCommands.RegisterAll (PLAN-0005) boot/mute/unmute/announce (MinorAdmin) and ban/unban/rolegrant/rolerevoke (FullAdmin), per ADR-0005. Shared AdminCommandHelpers covers the online-or-offline target lookup (live in World first, falling back to IThingRepository, never attaching an offline result into the world tree) and the combined online check (ConnectionState.Playing AND a connected Session - ConnectionState alone isn't enough, since it's Ignore'd by PlayerBehaviorConfiguration and defaults back to Playing on any freshly-repository-loaded PlayerBehavior), plus the grantable-role allowlist parse that rejects All/None even though both are literally named SecurityRole members. BootCommand/BanCommand call PlayerBehavior.MarkBooted() before disconnecting, so the target's own SessionLoop takes the immediate-removal path instead of Linkdead. BanCommand rejects self-targeting (no in-game recovery path); RoleRevokeCommand rejects revoking your own FullAdmin (same lockout risk) while still allowing every other revoke. AdminCommands.RegisterAll is not auto-wired into BuiltinCommands - a consumer opts in explicitly, passing their own IThingRepository. Co-Authored-By: Claude Sonnet 5 --- .../Builtin/Admin/AdminCommandHelpers.cs | 53 ++++++++ .../Commands/Builtin/Admin/AdminCommands.cs | 31 +++++ .../Commands/Builtin/Admin/AnnounceCommand.cs | 29 ++++ .../Commands/Builtin/Admin/BanCommand.cs | 60 ++++++++ .../Commands/Builtin/Admin/BootCommand.cs | 40 ++++++ .../Commands/Builtin/Admin/MuteCommand.cs | 37 +++++ .../Builtin/Admin/RoleGrantCommand.cs | 53 ++++++++ .../Builtin/Admin/RoleRevokeCommand.cs | 71 ++++++++++ .../Commands/Builtin/Admin/UnbanCommand.cs | 42 ++++++ .../Commands/Builtin/Admin/UnmuteCommand.cs | 37 +++++ .../Builtin/Admin/AnnounceCommandTests.cs | 46 +++++++ .../Commands/Builtin/Admin/BanCommandTests.cs | 87 ++++++++++++ .../Builtin/Admin/BootCommandTests.cs | 100 ++++++++++++++ .../Builtin/Admin/MuteCommandTests.cs | 79 +++++++++++ .../Builtin/Admin/RoleGrantCommandTests.cs | 87 ++++++++++++ .../Builtin/Admin/RoleRevokeCommandTests.cs | 128 ++++++++++++++++++ .../Builtin/Admin/UnbanCommandTests.cs | 37 +++++ .../Builtin/Admin/UnmuteCommandTests.cs | 56 ++++++++ .../Commands/HelpCommandTests.cs | 8 +- 19 files changed, 1077 insertions(+), 4 deletions(-) create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommands.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/BanCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/MuteCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/RoleGrantCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/RoleRevokeCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/UnbanCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/UnmuteCommand.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/AnnounceCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnbanCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnmuteCommandTests.cs diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs new file mode 100644 index 0000000..0815257 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs @@ -0,0 +1,53 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +// Shared by the moderation commands below - target lookup and "is this +// player actually online" both need the exact same combined check +// LoginFlow.LoginExistingAsync already established (ConnectionState alone +// isn't enough - it's Ignore'd by PlayerBehaviorConfiguration, so it +// defaults back to Playing on any freshly-repository-loaded PlayerBehavior, +// including one sitting in World with no live session at all). +internal static class AdminCommandHelpers +{ + /// Finds a player by username, live in the world first, then falling back to the repository (offline). Never attaches an offline result into the world tree. + public static async Task FindTargetAsync(IWorld world, IThingRepository repository, string username, CancellationToken ct) + { + var live = world.AllWithBehavior() + .FirstOrDefault(p => string.Equals(p.FindBehavior()!.Username, username, StringComparison.OrdinalIgnoreCase)); + if (live is not null) + return live; + + return await repository.FindPlayerByUsernameAsync(username, ct); + } + + /// Whether a player Thing is currently online - both and a connected , not either alone. + public static bool IsOnline(Thing player) + { + var behavior = player.FindBehavior(); + return behavior is { ConnectionState: var state, Session: { IsConnected: true } } && state == ConnectionState.Playing; + } + + /// + /// Parses an individually-grantable role name - rejects (every current and future flag, not a + /// real assignable tier) and (a + /// meaningless no-op), even though a plain Enum.TryParse would + /// accept both since they're literally named enum members. + /// + public static bool TryParseGrantableRole(string roleName, out SecurityRole role) + { + if (Enum.TryParse(roleName, ignoreCase: true, out SecurityRole parsed) + && parsed is not (SecurityRole.None or SecurityRole.All) + && Enum.IsDefined(parsed)) + { + role = parsed; + return true; + } + + role = SecurityRole.None; + return false; + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommands.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommands.cs new file mode 100644 index 0000000..8ca83a7 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommands.cs @@ -0,0 +1,31 @@ +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// Registers the moderation command set (ADR-0005) - day-to-day moderation +/// (boot/mute/unmute/announce) at , harder-to-reverse or +/// privilege-affecting actions (ban/unban/rolegrant/ +/// rolerevoke) at . Not called +/// automatically by - a consumer +/// calls this themselves (passing their own ) +/// from whichever callback they already pass into their ruleset's +/// registration entry point, the same way a consumer opts into any other +/// non-default command set. +/// +public static class AdminCommands +{ + public static void RegisterAll(ICommandRegistry registry, IThingRepository repository) + { + registry.RegisterWithRole(new BootCommand(), SecurityRole.MinorAdmin); + registry.RegisterWithRole(new MuteCommand(repository), SecurityRole.MinorAdmin); + registry.RegisterWithRole(new UnmuteCommand(repository), SecurityRole.MinorAdmin); + registry.RegisterWithRole(new AnnounceCommand(), SecurityRole.MinorAdmin); + + registry.RegisterWithRole(new BanCommand(repository), SecurityRole.FullAdmin); + registry.RegisterWithRole(new UnbanCommand(repository), SecurityRole.FullAdmin); + registry.RegisterWithRole(new RoleGrantCommand(repository), SecurityRole.FullAdmin); + registry.RegisterWithRole(new RoleRevokeCommand(repository), SecurityRole.FullAdmin); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs new file mode 100644 index 0000000..0cc6a6f --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs @@ -0,0 +1,29 @@ +using SharpMud.Engine.Behaviors; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// The announce command () - broadcasts to every currently-online player. +public sealed class AnnounceCommand : ICommand +{ + public string Verb => "announce"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Announce what?", ct)) + return; + + var message = string.Join(' ', ctx.Args); + + // Deliberately not WhoCommand's world.AllWithBehavior() + // with no further filter - that also matches Linkdead players + // (ADR-0004), which would attempt a write to their stale session. + foreach (var player in ctx.World.AllWithBehavior()) + { + if (!AdminCommandHelpers.IsOnline(player)) + continue; + + await player.FindBehavior()!.Session!.WriteLineAsync($"[Announcement] {message}", ct); + } + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/BanCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/BanCommand.cs new file mode 100644 index 0000000..cba8a3f --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/BanCommand.cs @@ -0,0 +1,60 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// The ban command () - sets +/// IsBanned on an online-or-offline target, saved immediately, and +/// disconnects the target immediately if they're currently online +/// (SessionLoop never re-checks IsBanned mid-session). +/// Rejects self-targeting - a ban has no in-game recovery path. +/// +public sealed class BanCommand : ICommand +{ + private readonly IThingRepository _repository; + + public BanCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "ban"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Ban whom?", ct)) + return; + + var username = string.Join(' ', ctx.Args); + var actorUsername = ctx.Actor.FindBehavior()?.Username; + if (string.Equals(actorUsername, username, StringComparison.OrdinalIgnoreCase)) + { + await ctx.Session.WriteLineAsync("You cannot ban yourself.", ct); + return; + } + + var target = await AdminCommandHelpers.FindTargetAsync(ctx.World, _repository, username, ct); + if (target is null) + { + await ctx.Session.WriteLineAsync($"No player named {username} was found.", ct); + return; + } + + var targetBehavior = target.FindBehavior()!; + targetBehavior.Ban(); + + if (AdminCommandHelpers.IsOnline(target)) + { + targetBehavior.MarkBooted(); + var targetSession = targetBehavior.Session!; + await targetSession.WriteLineAsync("You have been banned by an administrator.", ct); + await targetSession.DisconnectAsync("Banned by an administrator.", ct); + } + + await _repository.SaveTreeAsync(target, ct); + + await ctx.Session.WriteLineAsync($"You banned {target.Name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs new file mode 100644 index 0000000..1b7af7c --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs @@ -0,0 +1,40 @@ +using SharpMud.Engine.Behaviors; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// The boot command () - disconnects a currently-online target. +public sealed class BootCommand : ICommand +{ + public string Verb => "boot"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Boot whom?", ct)) + return; + + var username = string.Join(' ', ctx.Args); + var target = ctx.World.AllWithBehavior() + .FirstOrDefault(p => string.Equals(p.FindBehavior()!.Username, username, StringComparison.OrdinalIgnoreCase)); + + if (target is null || !AdminCommandHelpers.IsOnline(target)) + { + await ctx.Session.WriteLineAsync($"{username} is not online.", ct); + return; + } + + var targetBehavior = target.FindBehavior()!; + var targetSession = targetBehavior.Session!; + + // Before disconnecting - without this, the target's own SessionLoop + // sees an ordinary dropped connection and takes the Linkdead path + // (ADR-0004), letting them just reconnect and resume where they + // were, making boot a no-op as a moderation tool. + targetBehavior.MarkBooted(); + + await targetSession.WriteLineAsync("You have been disconnected by an administrator.", ct); + await targetSession.DisconnectAsync("Booted by an administrator.", ct); + + await ctx.Session.WriteLineAsync($"You booted {target.Name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/MuteCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/MuteCommand.cs new file mode 100644 index 0000000..848741a --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/MuteCommand.cs @@ -0,0 +1,37 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// The mute command () - sets IsMuted on an online-or-offline target, saved immediately. +public sealed class MuteCommand : ICommand +{ + private readonly IThingRepository _repository; + + public MuteCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "mute"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Mute whom?", ct)) + return; + + var username = string.Join(' ', ctx.Args); + var target = await AdminCommandHelpers.FindTargetAsync(ctx.World, _repository, username, ct); + if (target is null) + { + await ctx.Session.WriteLineAsync($"No player named {username} was found.", ct); + return; + } + + target.FindBehavior()!.Mute(); + await _repository.SaveTreeAsync(target, ct); + + await ctx.Session.WriteLineAsync($"You muted {target.Name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/RoleGrantCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/RoleGrantCommand.cs new file mode 100644 index 0000000..7af2a75 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/RoleGrantCommand.cs @@ -0,0 +1,53 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// The rolegrant <username> <role> command () - grants a role to an online-or-offline +/// target via (accumulation happens +/// inside itself, not here), saved +/// immediately. Gated at specifically +/// so a can never self-escalate. +/// +public sealed class RoleGrantCommand : ICommand +{ + private readonly IThingRepository _repository; + + public RoleGrantCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "rolegrant"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args.Count < 2) + { + await ctx.Session.WriteLineAsync("Usage: rolegrant ", ct); + return; + } + + var username = ctx.Args[0]; + if (!AdminCommandHelpers.TryParseGrantableRole(ctx.Args[1], out var role)) + { + await ctx.Session.WriteLineAsync($"'{ctx.Args[1]}' isn't a grantable role.", ct); + return; + } + + var target = await AdminCommandHelpers.FindTargetAsync(ctx.World, _repository, username, ct); + if (target is null) + { + await ctx.Session.WriteLineAsync($"No player named {username} was found.", ct); + return; + } + + target.FindBehavior()!.GrantRole(role); + await _repository.SaveTreeAsync(target, ct); + + await ctx.Session.WriteLineAsync($"Granted {role} to {target.Name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/RoleRevokeCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/RoleRevokeCommand.cs new file mode 100644 index 0000000..18dea96 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/RoleRevokeCommand.cs @@ -0,0 +1,71 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// The rolerevoke <username> <role> command () - revokes a role from an +/// online-or-offline target via +/// (hierarchy-invariant enforcement happens inside itself, not here), saved immediately. Rejects +/// revoking your own - same class of +/// lockout risk as 's self-targeting guard: a sole +/// revoking their own tier has no +/// in-game path back without another already present to re-grant it. +/// Revoking any other role from yourself, or from someone else, is unaffected. +/// +public sealed class RoleRevokeCommand : ICommand +{ + private readonly IThingRepository _repository; + + public RoleRevokeCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "rolerevoke"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args.Count < 2) + { + await ctx.Session.WriteLineAsync("Usage: rolerevoke ", ct); + return; + } + + var username = ctx.Args[0]; + if (!AdminCommandHelpers.TryParseGrantableRole(ctx.Args[1], out var role)) + { + await ctx.Session.WriteLineAsync($"'{ctx.Args[1]}' isn't a revocable role.", ct); + return; + } + + var actorUsername = ctx.Actor.FindBehavior()?.Username; + if (role == SecurityRole.FullAdmin && string.Equals(actorUsername, username, StringComparison.OrdinalIgnoreCase)) + { + await ctx.Session.WriteLineAsync("You cannot revoke your own FullAdmin.", ct); + return; + } + + var target = await AdminCommandHelpers.FindTargetAsync(ctx.World, _repository, username, ct); + if (target is null) + { + await ctx.Session.WriteLineAsync($"No player named {username} was found.", ct); + return; + } + + var failure = target.FindBehavior()!.RevokeRole(role); + if (failure is not null) + { + await ctx.Session.WriteLineAsync(failure, ct); + return; + } + + await _repository.SaveTreeAsync(target, ct); + + await ctx.Session.WriteLineAsync($"Revoked {role} from {target.Name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/UnbanCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/UnbanCommand.cs new file mode 100644 index 0000000..702921b --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/UnbanCommand.cs @@ -0,0 +1,42 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// The unban command () - clears +/// IsBanned on an online-or-offline target, saved immediately. No +/// self-targeting guard needed, unlike - undoing +/// your own ban isn't reachable, since you can't be logged in while banned. +/// +public sealed class UnbanCommand : ICommand +{ + private readonly IThingRepository _repository; + + public UnbanCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "unban"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Unban whom?", ct)) + return; + + var username = string.Join(' ', ctx.Args); + var target = await AdminCommandHelpers.FindTargetAsync(ctx.World, _repository, username, ct); + if (target is null) + { + await ctx.Session.WriteLineAsync($"No player named {username} was found.", ct); + return; + } + + target.FindBehavior()!.Unban(); + await _repository.SaveTreeAsync(target, ct); + + await ctx.Session.WriteLineAsync($"You unbanned {target.Name}.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/UnmuteCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/UnmuteCommand.cs new file mode 100644 index 0000000..19e6dcf --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/UnmuteCommand.cs @@ -0,0 +1,37 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Core; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// The unmute command () - clears IsMuted on an online-or-offline target, saved immediately. +public sealed class UnmuteCommand : ICommand +{ + private readonly IThingRepository _repository; + + public UnmuteCommand(IThingRepository repository) + { + _repository = repository; + } + + public string Verb => "unmute"; + public IReadOnlyList Aliases { get; } = []; + + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (await CommandGuards.RequireArgsAsync(ctx, "Unmute whom?", ct)) + return; + + var username = string.Join(' ', ctx.Args); + var target = await AdminCommandHelpers.FindTargetAsync(ctx.World, _repository, username, ct); + if (target is null) + { + await ctx.Session.WriteLineAsync($"No player named {username} was found.", ct); + return; + } + + target.FindBehavior()!.Unmute(); + await _repository.SaveTreeAsync(target, ct); + + await ctx.Session.WriteLineAsync($"You unmuted {target.Name}.", ct); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/AnnounceCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/AnnounceCommandTests.cs new file mode 100644 index 0000000..d8eff24 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/AnnounceCommandTests.cs @@ -0,0 +1,46 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class AnnounceCommandTests +{ + [Fact] + public async Task ExecuteAsync_BroadcastsToEveryOnlinePlayer_ButNotLinkdeadOrSessionlessOnes() + { + var adminSession = Substitute.For(); + adminSession.IsConnected.Returns(true); + var onlineSession = Substitute.For(); + onlineSession.IsConnected.Returns(true); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash", Session = adminSession }); + world.Register(admin); + + var online = new Thing { Id = ThingId.New(), Name = "Online" }; + online.Behaviors.Add(new PlayerBehavior { Username = "Online", PasswordHash = "test-hash", Session = onlineSession }); + world.Register(online); + + var linkdeadBehavior = new PlayerBehavior { Username = "Linkdead", PasswordHash = "test-hash" }; + linkdeadBehavior.EnterLinkdead(DateTimeOffset.UtcNow); + var linkdead = new Thing { Id = ThingId.New(), Name = "Linkdead" }; + linkdead.Behaviors.Add(linkdeadBehavior); + world.Register(linkdead); + + var sessionless = new Thing { Id = ThingId.New(), Name = "Sessionless" }; + sessionless.Behaviors.Add(new PlayerBehavior { Username = "Sessionless", PasswordHash = "test-hash" }); + world.Register(sessionless); + + var sut = new AnnounceCommand(); + var ctx = new CommandContext(admin, admin, ["Server", "restarting", "soon"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("[Announcement] Server restarting soon", Arg.Any()); + await onlineSession.Received(1).WriteLineAsync("[Announcement] Server restarting soon", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs new file mode 100644 index 0000000..64bf70e --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs @@ -0,0 +1,87 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class BanCommandTests +{ + [Fact] + public async Task ExecuteAsync_RejectsSelfTargeting_AndLeavesIsBannedUnchanged() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var adminBehavior = new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash", Session = adminSession }; + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(adminBehavior); + world.Register(admin); + + var sut = new BanCommand(repository); + var ctx = new CommandContext(admin, admin, ["Admin"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + adminBehavior.IsBanned.Should().BeFalse(); + await adminSession.Received(1).WriteLineAsync("You cannot ban yourself.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + } + + [Fact] + public async Task ExecuteAsync_BansAndDisconnects_WhenTargetingAnotherOnlinePlayer() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var targetSession = Substitute.For(); + targetSession.IsConnected.Returns(true); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash", Session = adminSession }); + world.Register(admin); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash", Session = targetSession }; + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + world.Register(target); + + var sut = new BanCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.IsBanned.Should().BeTrue(); + targetBehavior.WasBooted.Should().BeTrue(); + await targetSession.Received(1).DisconnectAsync(Arg.Any(), Arg.Any()); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_BansWithoutDisconnecting_WhenTargetIsOffline() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash", Session = adminSession }); + world.Register(admin); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }; + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + repository.FindPlayerByUsernameAsync("Target", Arg.Any()).Returns(target); + + var sut = new BanCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.IsBanned.Should().BeTrue(); + targetBehavior.WasBooted.Should().BeFalse(); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs new file mode 100644 index 0000000..fc130f0 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs @@ -0,0 +1,100 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class BootCommandTests +{ + private static Thing MakePlayer(string username, ISession? session, ConnectionState? connectionState = null) + { + var thing = new Thing { Id = ThingId.New(), Name = username }; + var behavior = new PlayerBehavior { Username = username, PasswordHash = "test-hash", Session = session }; + if (connectionState == ConnectionState.Linkdead) + behavior.EnterLinkdead(DateTimeOffset.UtcNow); + thing.Behaviors.Add(behavior); + return thing; + } + + [Fact] + public async Task ExecuteAsync_MarksBootedAndDisconnects_WhenTargetIsOnline() + { + var adminSession = Substitute.For(); + var targetSession = Substitute.For(); + targetSession.IsConnected.Returns(true); + + var world = new World(); + var admin = MakePlayer("Admin", adminSession); + var target = MakePlayer("Target", targetSession); + world.Register(admin); + world.Register(target); + + var sut = new BootCommand(); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + target.FindBehavior()!.WasBooted.Should().BeTrue(); + await targetSession.Received(1).DisconnectAsync(Arg.Any(), Arg.Any()); + await adminSession.Received(1).WriteLineAsync("You booted Target.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotOnlineMessage_WhenTargetIsLinkdead() + { + var adminSession = Substitute.For(); + var targetSession = Substitute.For(); + + var world = new World(); + var admin = MakePlayer("Admin", adminSession); + var target = MakePlayer("Target", targetSession, ConnectionState.Linkdead); + world.Register(admin); + world.Register(target); + + var sut = new BootCommand(); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + target.FindBehavior()!.WasBooted.Should().BeFalse(); + await targetSession.DidNotReceiveWithAnyArgs().DisconnectAsync(default!, default); + await adminSession.Received(1).WriteLineAsync("Target is not online.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotOnlineMessage_WhenTargetHasNoSession() + { + var adminSession = Substitute.For(); + + var world = new World(); + var admin = MakePlayer("Admin", adminSession); + var target = MakePlayer("Target", session: null); + world.Register(admin); + world.Register(target); + + var sut = new BootCommand(); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("Target is not online.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotOnlineMessage_WhenTargetDoesNotExist() + { + var adminSession = Substitute.For(); + var world = new World(); + var admin = MakePlayer("Admin", adminSession); + world.Register(admin); + + var sut = new BootCommand(); + var ctx = new CommandContext(admin, admin, ["Ghost"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("Ghost is not online.", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs new file mode 100644 index 0000000..c0a2495 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs @@ -0,0 +1,79 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class MuteCommandTests +{ + [Fact] + public async Task ExecuteAsync_MutesAndSaves_WhenTargetIsLiveInWorld() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }); + world.Register(target); + + var sut = new MuteCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + target.FindBehavior()!.IsMuted.Should().BeTrue(); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + await adminSession.Received(1).WriteLineAsync("You muted Target.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_MutesAndSaves_WhenTargetIsOfflineButInRepository() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }); + repository.FindPlayerByUsernameAsync("Target", Arg.Any()).Returns(target); + + var sut = new MuteCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + target.FindBehavior()!.IsMuted.Should().BeTrue(); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotFoundMessage_WhenTargetDoesNotExistAnywhere() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var sut = new MuteCommand(repository); + var ctx = new CommandContext(admin, admin, ["Ghost"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("No player named Ghost was found.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs new file mode 100644 index 0000000..82b2276 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs @@ -0,0 +1,87 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class RoleGrantCommandTests +{ + private static (Thing Admin, ISession AdminSession, World World) MakeAdmin() + { + var adminSession = Substitute.For(); + var world = new World(); + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + return (admin, adminSession, world); + } + + [Fact] + public async Task ExecuteAsync_GrantsRoleAndSaves_WhenTargetExistsAndRoleIsValid() + { + var repository = Substitute.For(); + var (admin, adminSession, world) = MakeAdmin(); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }; + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + world.Register(target); + + var sut = new RoleGrantCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target", "MinorAdmin"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.Roles.Should().HaveFlag(SecurityRole.MinorAdmin); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + } + + [Theory] + [InlineData("All")] + [InlineData("all")] + [InlineData("None")] + [InlineData("none")] + public async Task ExecuteAsync_RejectsAllAndNone_RegardlessOfCasing(string roleName) + { + var repository = Substitute.For(); + var (admin, adminSession, world) = MakeAdmin(); + + var sut = new RoleGrantCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target", roleName], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await repository.DidNotReceiveWithAnyArgs().FindPlayerByUsernameAsync(default!, default); + await adminSession.Received(1).WriteLineAsync($"'{roleName}' isn't a grantable role.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsUnknownRoleName() + { + var repository = Substitute.For(); + var (admin, adminSession, world) = MakeAdmin(); + + var sut = new RoleGrantCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target", "Wizard"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("'Wizard' isn't a grantable role.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsUsageMessage_WhenMissingArguments() + { + var repository = Substitute.For(); + var (admin, adminSession, world) = MakeAdmin(); + + var sut = new RoleGrantCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("Usage: rolegrant ", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs new file mode 100644 index 0000000..2e6e69f --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs @@ -0,0 +1,128 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class RoleRevokeCommandTests +{ + [Fact] + public async Task ExecuteAsync_RejectsRevokingOwnFullAdmin() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var adminBehavior = new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }; + adminBehavior.GrantRole(SecurityRole.FullAdmin); + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(adminBehavior); + world.Register(admin); + + var sut = new RoleRevokeCommand(repository); + var ctx = new CommandContext(admin, admin, ["Admin", "FullAdmin"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + adminBehavior.Roles.Should().HaveFlag(SecurityRole.FullAdmin); + await adminSession.Received(1).WriteLineAsync("You cannot revoke your own FullAdmin.", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + } + + [Fact] + public async Task ExecuteAsync_AllowsRevokingADifferentRoleFromSelf() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var adminBehavior = new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }; + adminBehavior.GrantRole(SecurityRole.Helper); + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(adminBehavior); + world.Register(admin); + + var sut = new RoleRevokeCommand(repository); + var ctx = new CommandContext(admin, admin, ["Admin", "Helper"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + adminBehavior.Roles.Should().NotHaveFlag(SecurityRole.Helper); + await repository.Received(1).SaveTreeAsync(admin, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_AllowsRevokingFullAdminFromSomeoneElse() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }; + targetBehavior.GrantRole(SecurityRole.FullAdmin); + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + world.Register(target); + + var sut = new RoleRevokeCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target", "FullAdmin"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.Roles.Should().NotHaveFlag(SecurityRole.FullAdmin); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RelaysHierarchyFailure_WithoutSaving() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }; + targetBehavior.GrantRole(SecurityRole.FullAdmin); + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + world.Register(target); + + var sut = new RoleRevokeCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target", "MinorAdmin"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.Roles.Should().HaveFlag(SecurityRole.MinorAdmin); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + await adminSession.Received(1).WriteLineAsync( + Arg.Is(s => s!.Contains("FullAdmin")), Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_RejectsAllAndNoneRoleNames() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var sut = new RoleRevokeCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target", "All"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("'All' isn't a revocable role.", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnbanCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnbanCommandTests.cs new file mode 100644 index 0000000..de3d527 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnbanCommandTests.cs @@ -0,0 +1,37 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class UnbanCommandTests +{ + [Fact] + public async Task ExecuteAsync_UnbansAndSaves_WhenTargetExists() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }; + targetBehavior.Ban(); + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + repository.FindPlayerByUsernameAsync("Target", Arg.Any()).Returns(target); + + var sut = new UnbanCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.IsBanned.Should().BeFalse(); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + await adminSession.Received(1).WriteLineAsync("You unbanned Target.", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnmuteCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnmuteCommandTests.cs new file mode 100644 index 0000000..53bf4d9 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/UnmuteCommandTests.cs @@ -0,0 +1,56 @@ +using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class UnmuteCommandTests +{ + [Fact] + public async Task ExecuteAsync_UnmutesAndSaves_WhenTargetIsLiveInWorld() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var targetBehavior = new PlayerBehavior { Username = "Target", PasswordHash = "test-hash" }; + targetBehavior.Mute(); + var target = new Thing { Id = ThingId.New(), Name = "Target" }; + target.Behaviors.Add(targetBehavior); + world.Register(target); + + var sut = new UnmuteCommand(repository); + var ctx = new CommandContext(admin, admin, ["Target"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + targetBehavior.IsMuted.Should().BeFalse(); + await repository.Received(1).SaveTreeAsync(target, Arg.Any()); + await adminSession.Received(1).WriteLineAsync("You unmuted Target.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsNotFoundMessage_WhenTargetDoesNotExistAnywhere() + { + var repository = Substitute.For(); + var adminSession = Substitute.For(); + var world = new World(); + + var admin = new Thing { Id = ThingId.New(), Name = "Admin" }; + admin.Behaviors.Add(new PlayerBehavior { Username = "Admin", PasswordHash = "test-hash" }); + world.Register(admin); + + var sut = new UnmuteCommand(repository); + var ctx = new CommandContext(admin, admin, ["Ghost"], world, adminSession); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await adminSession.Received(1).WriteLineAsync("No player named Ghost was found.", Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs index f40800c..12d3a4c 100644 --- a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs @@ -38,8 +38,8 @@ public async Task ExecuteAsync_OmitsRoleGatedCommand_WhenActorLacksTheRequiredRo await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - await session.DidNotReceive().WriteLineAsync(Arg.Is(s => s.Contains("ban")), Arg.Any()); - await session.Received(1).WriteLineAsync(Arg.Is(s => s.Contains("look")), Arg.Any()); + await session.DidNotReceive().WriteLineAsync(Arg.Is(s => s!.Contains("ban")), Arg.Any()); + await session.Received(1).WriteLineAsync(Arg.Is(s => s!.Contains("look")), Arg.Any()); } [Fact] @@ -55,7 +55,7 @@ public async Task ExecuteAsync_IncludesRoleGatedCommand_WhenActorHasTheRequiredR await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - await session.Received(1).WriteLineAsync(Arg.Is(s => s.Contains("ban")), Arg.Any()); + await session.Received(1).WriteLineAsync(Arg.Is(s => s!.Contains("ban")), Arg.Any()); } [Fact] @@ -71,6 +71,6 @@ public async Task ExecuteAsync_AlwaysIncludesNonGatedCommands_RegardlessOfRole() await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - await session.Received(1).WriteLineAsync(Arg.Is(s => s.Contains("look")), Arg.Any()); + await session.Received(1).WriteLineAsync(Arg.Is(s => s!.Contains("look")), Arg.Any()); } } From 78478f7b9ee34107e9097d46c9860603f301c777 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 13:57:43 -0400 Subject: [PATCH 04/10] feat: bootstrap initial admin and enforce ban/boot in login+session flow LoginFlow grants SHARPMUD_INITIAL_ADMIN FullAdmin on login (idempotent), rejects banned accounts. SessionLoop treats WasBooted like explicitQuit so boot/ban actually disconnect instead of resuming via Linkdead. --- src/SharpMud.Hosting/LoginFlow.cs | 42 ++++++++++++++++++- src/SharpMud.Hosting/SessionLoop.cs | 28 +++++++++---- src/SharpMud.Hosting/SharpMudHostOptions.cs | 10 ++++- .../SharpMud.Hosting.Tests/LoginFlowTests.cs | 5 ++- .../ThingRepositoryTests.cs | 29 +++++++++++++ 5 files changed, 100 insertions(+), 14 deletions(-) diff --git a/src/SharpMud.Hosting/LoginFlow.cs b/src/SharpMud.Hosting/LoginFlow.cs index fd1d991..e573a17 100644 --- a/src/SharpMud.Hosting/LoginFlow.cs +++ b/src/SharpMud.Hosting/LoginFlow.cs @@ -1,4 +1,5 @@ using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; using SharpMud.Engine.Core; using SharpMud.Engine.Sessions; @@ -17,12 +18,14 @@ public sealed class LoginFlow private readonly WorldContext _worldContext; private readonly IThingRepository _repository; private readonly IPlayerFactory _playerFactory; + private readonly SharpMudHostOptions _hostOptions; - public LoginFlow(WorldContext worldContext, IThingRepository repository, IPlayerFactory playerFactory) + public LoginFlow(WorldContext worldContext, IThingRepository repository, IPlayerFactory playerFactory, SharpMudHostOptions hostOptions) { _worldContext = worldContext; _repository = repository; _playerFactory = playerFactory; + _hostOptions = hostOptions; } // Returns null if the connection should be dropped (empty input, @@ -44,13 +47,40 @@ public LoginFlow(WorldContext worldContext, IThingRepository repository, IPlayer : await MaybeCreateAsync(session, username, ct); if (player is not null) + { + await MaybeGrantInitialAdminAsync(player, ct); return player; + } if (!session.IsConnected) return null; } } + // ADR-0005 bootstrap - checked here, not at boot time and again at + // character creation, since a single check that runs on every login + // (new character or existing) covers the fresh-server case, the + // restart case, and "the env var was set after this character already + // existed," identically. Idempotent (the FullAdmin check below), so + // safe to run on every login for the matching username, not just the + // first. + private async Task MaybeGrantInitialAdminAsync(Thing player, CancellationToken ct) + { + var username = _hostOptions.InitialAdminUsername; + if (username is null) + return; + + var playerBehavior = player.FindBehavior()!; + if (!string.Equals(playerBehavior.Username, username, StringComparison.OrdinalIgnoreCase)) + return; + + if ((playerBehavior.Roles & SecurityRole.FullAdmin) != SecurityRole.None) + return; + + playerBehavior.GrantRole(SecurityRole.FullAdmin); + await _repository.SaveTreeAsync(player, ct); + } + private async Task FindAndAttachExistingAsync(string username, CancellationToken ct) { var world = _worldContext.World; @@ -97,6 +127,16 @@ public LoginFlow(WorldContext worldContext, IThingRepository repository, IPlayer continue; } + // ADR-0005 - checked right after password verification, before + // the ConnectionState branch below, so a banned user gets a + // distinct message rather than silently falling through to a + // reconnect/already-logged-in check that doesn't apply to them. + if (playerBehavior.IsBanned) + { + await session.WriteLineAsync("This account has been banned.", ct); + return null; + } + // Unchanged from before ADR-0004: still actively Playing with a // live session means someone else is using this character right // now - reject, don't steal the session. diff --git a/src/SharpMud.Hosting/SessionLoop.cs b/src/SharpMud.Hosting/SessionLoop.cs index 6e9984d..e557606 100644 --- a/src/SharpMud.Hosting/SessionLoop.cs +++ b/src/SharpMud.Hosting/SessionLoop.cs @@ -88,6 +88,16 @@ public async Task RunAsync(ISession session, Thing player, CancellationToken ct) { var playerBehavior = player.FindBehavior(); + // WasBooted (ADR-0005) means an admin's BootCommand/BanCommand + // - running on a completely different connection's call stack - + // already decided this disconnect is intentional. Treated + // exactly like explicitQuit here: without this, this player's + // own SessionLoop can't tell an admin-triggered disconnect apart + // from an ordinary dropped connection, so it would take the + // Linkdead branch below and let them just reconnect and resume, + // making boot/ban a no-op as a moderation tool. + var wasIntentionalDisconnect = explicitQuit || (playerBehavior?.WasBooted ?? false); + // Guard + do this BEFORE the awaited save below (PR #1 review) - // if a reconnect races in while this disconnect is being // processed, LoginFlow must see Linkdead immediately, not a @@ -95,10 +105,10 @@ public async Task RunAsync(ISession session, Thing player, CancellationToken ct) // playerBehavior.Session == session check additionally backs off // entirely if a newer session already took over this same Thing // by the time we get here, so this disconnect can't clobber an - // already-active reconnect. explicitQuit's removal deliberately - // does NOT happen here (see below) - only the Linkdead - // transition needs to race ahead of the save. - if (!explicitQuit && playerBehavior?.Session == session) + // already-active reconnect. The removal below deliberately does + // NOT happen here (see below) - only the Linkdead transition + // needs to race ahead of the save. + if (!wasIntentionalDisconnect && playerBehavior?.Session == session) { // Linkdead, not an immediate world removal (ADR-0004) - the // Thing stays live in its room so LoginFlow can reconnect a @@ -116,12 +126,12 @@ public async Task RunAsync(ISession session, Thing player, CancellationToken ct) // cancellation mid-operation. await _repository.SaveTreeAsync(player, CancellationToken.None); - // explicitQuit's removal happens AFTER the save, not before - // (PR #1 review) - ThingRepository.SaveTreeAsync persists - // ParentId from thing.Parent at save time, so removing first - // would save ParentId=null and lose the room a player quit + // Removal happens AFTER the save, not before (PR #1 review) - + // ThingRepository.SaveTreeAsync persists ParentId from + // thing.Parent at save time, so removing first would save + // ParentId=null and lose the room a player quit/was booted // from. Same session-identity guard as above, for consistency. - if (explicitQuit && playerBehavior?.Session == session) + if (wasIntentionalDisconnect && playerBehavior?.Session == session) { player.Parent?.Remove(player); world.Unregister(player.Id); diff --git a/src/SharpMud.Hosting/SharpMudHostOptions.cs b/src/SharpMud.Hosting/SharpMudHostOptions.cs index 66e0608..4cb5794 100644 --- a/src/SharpMud.Hosting/SharpMudHostOptions.cs +++ b/src/SharpMud.Hosting/SharpMudHostOptions.cs @@ -9,12 +9,18 @@ namespace SharpMud.Hosting; // Named SharpMudHostOptions, not HostOptions - the latter collides with // Microsoft.Extensions.Hosting.HostOptions, a real BCL type every consumer // of this package will also have in scope. -public sealed record SharpMudHostOptions(string DbPath) +public sealed record SharpMudHostOptions(string DbPath, string? InitialAdminUsername = null) { public static SharpMudHostOptions Parse(IReadOnlyDictionary env) { var dbPath = env.GetValueOrDefault("SHARPMUD_DB_PATH") ?? "./sharpmud.db"; - return new SharpMudHostOptions(dbPath); + // ADR-0005 bootstrap - the only path to a FullAdmin on a fresh + // deployment, since granting a role itself requires FullAdmin. + // Consumed by LoginFlow, not parsed/threaded through per-connection + // context types (see docs/plans/0005-security-role-model-and-moderation-commands.md). + var initialAdminUsername = env.GetValueOrDefault("SHARPMUD_INITIAL_ADMIN"); + + return new SharpMudHostOptions(dbPath, initialAdminUsername); } } diff --git a/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs b/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs index ee57fdf..dc09d84 100644 --- a/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs +++ b/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs @@ -9,14 +9,15 @@ public sealed class LoginFlowTests { private static Thing MakeRoom() => new() { Id = ThingId.New(), Name = "Room" }; - private static (LoginFlow loginFlow, IThingRepository repository) MakeLoginFlow(World world, Thing room) + private static (LoginFlow loginFlow, IThingRepository repository) MakeLoginFlow(World world, Thing room, string? initialAdminUsername = null) { var worldContext = new WorldContext(); worldContext.Initialize(world, room, room); var repository = Substitute.For(); var playerFactory = Substitute.For(); + var hostOptions = new SharpMudHostOptions("unused.db", initialAdminUsername); - return (new LoginFlow(worldContext, repository, playerFactory), repository); + return (new LoginFlow(worldContext, repository, playerFactory, hostOptions), repository); } [Fact] diff --git a/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs b/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs index 23c47cc..9e1c8d4 100644 --- a/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs +++ b/tests/SharpMud.Persistence.Tests/ThingRepositoryTests.cs @@ -1,4 +1,5 @@ using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; using SharpMud.Engine.Core; using SharpMud.Persistence.Tests.TestKit; using SharpMud.Ruleset.Rpg; @@ -106,6 +107,34 @@ public async Task SaveTreeAsync_ThenLoadTreeAsync_RoundTripsPlayerWithStatsAndCa carriedItem.HasBehavior().Should().BeTrue(); } + [Fact] + public async Task SaveTreeAsync_ThenLoadTreeAsync_RoundTripsRolesAndMuteAndBanState() + { + var playerBehavior = new PlayerBehavior { Username = "Moderated", PasswordHash = "test-hash" }; + playerBehavior.GrantRole(SecurityRole.FullAdmin); + playerBehavior.Mute(); + playerBehavior.Ban(); + + var player = new Thing { Id = ThingId.New(), Name = "Moderated" }; + player.Behaviors.Add(playerBehavior); + + await _sut.SaveTreeAsync(player, TestContext.Current.CancellationToken); + + var loaded = await _sut.LoadTreeAsync(player.Id, TestContext.Current.CancellationToken); + + var loadedBehavior = loaded!.FindBehavior(); + loadedBehavior.Should().NotBeNull(); + loadedBehavior!.Roles.Should().HaveFlag(SecurityRole.FullAdmin); + loadedBehavior.Roles.Should().HaveFlag(SecurityRole.MinorAdmin); + loadedBehavior.Roles.Should().HaveFlag(SecurityRole.Player); + loadedBehavior.IsMuted.Should().BeTrue(); + loadedBehavior.IsBanned.Should().BeTrue(); + + // WasBooted is transient (like Session/ConnectionState) - never + // meant to survive a save/load round trip. + loadedBehavior.WasBooted.Should().BeFalse(); + } + [Fact] public async Task FindPlayerByUsernameAsync_ReturnsMatchingPlayer() { From 56da0dbf4dd311a854c9c827b2bf169b0f82067a Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 13:58:20 -0400 Subject: [PATCH 05/10] feat: wire SHARPMUD_INITIAL_ADMIN and moderation commands into Classic sample Registers SharpMudHostOptions (LoginFlow needs it for bootstrap) and AdminCommands.RegisterAll via the Rpg ruleset's registerConsumerCommands callback, so boot/mute/ban/rolegrant are actually reachable in-game. --- samples/SharpMud.Samples.Classic/Program.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/samples/SharpMud.Samples.Classic/Program.cs b/samples/SharpMud.Samples.Classic/Program.cs index 707cf40..29cfd4a 100644 --- a/samples/SharpMud.Samples.Classic/Program.cs +++ b/samples/SharpMud.Samples.Classic/Program.cs @@ -5,6 +5,8 @@ using Serilog; using SharpMud.Adapters.Cli; using SharpMud.Adapters.Telnet; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; using SharpMud.Hosting; using SharpMud.Persistence; using SharpMud.Persistence.Sqlite; @@ -48,9 +50,15 @@ var env = new Dictionary { ["SHARPMUD_DB_PATH"] = dbPathArg ?? Environment.GetEnvironmentVariable("SHARPMUD_DB_PATH"), + ["SHARPMUD_INITIAL_ADMIN"] = Environment.GetEnvironmentVariable("SHARPMUD_INITIAL_ADMIN"), }; var hostOptions = SharpMudHostOptions.Parse(env); +// LoginFlow consumes this directly (ADR-0005 bootstrap) - not otherwise +// registered by SharpMud.Hosting itself, since it's parsed here in the +// consumer's own composition root alongside every other env-var option. +builder.Services.AddSingleton(hostOptions); + builder.Services.AddSharpMudSqlitePersistence(hostOptions.DbPath); builder.Services.AddSingleton(); builder.Services.AddSharpMudWorld(); @@ -59,10 +67,11 @@ // SharpMud.Ruleset.Rpg's combat scaffolding (ICombatResolver, ICombatManager // as both itself and ITickable, the dice service, its own // IBehaviorMappingContributor, and the kill/attack/flee commands) - see -// docs/adr/0008-ruleset-scaffolding-tier.md. Classic has no commands of its -// own beyond what Rpg already provides, so no registerConsumerCommands -// callback is needed here. -builder.Services.AddSharpMudRpgRuleset(); +// docs/adr/0008-ruleset-scaffolding-tier.md. The registerConsumerCommands +// callback wires AdminCommands (ADR-0005 moderation) - Rpg has no notion of +// security roles itself, so this is Classic's own composition-root concern. +builder.Services.AddSharpMudRpgRuleset((sp, registry) => + AdminCommands.RegisterAll(registry, sp.GetRequiredService())); // Transport mode: SHARPMUD_MODE/SHARPMUD_TELNET_PORT/--telnet, same // precedence as before (CLI arg wins over env var) - this is now the From 8cc902b12a9f8237c6f3f6a4718b947052c283bf Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 13:59:34 -0400 Subject: [PATCH 06/10] test: cover bootstrap admin grant, ban rejection, and WasBooted removal LoginFlow: initial-admin grant (match/no-match/null/idempotent) and banned-account rejection. SessionLoop: WasBooted takes the same immediate- removal path as explicitQuit. SharpMudHostOptions: SHARPMUD_INITIAL_ADMIN parsing. --- .../SharpMud.Hosting.Tests/LoginFlowTests.cs | 130 ++++++++++++++++++ .../SessionLoopTests.cs | 27 ++++ .../SharpMudHostOptionsTests.cs | 18 +++ 3 files changed, 175 insertions(+) diff --git a/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs b/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs index dc09d84..053bf75 100644 --- a/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs +++ b/tests/SharpMud.Hosting.Tests/LoginFlowTests.cs @@ -1,4 +1,5 @@ using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Commands; using SharpMud.Engine.Core; using SharpMud.Engine.Sessions; using SharpMud.Hosting; @@ -136,4 +137,133 @@ public async Task RunAsync_RejectsAsExpired_WhenLinkdeadThingAlreadyRemovedFromW result.Should().BeNull(); await session.Received(1).WriteLineAsync("That session has expired. Please log in again.", Arg.Any()); } + + [Fact] + public async Task RunAsync_Rejects_WhenAccountIsBanned() + { + var world = new World(); + var room = MakeRoom(); + world.Register(room); + + var passwordHash = PasswordHashing.Hash("correct-horse"); + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + var playerBehavior = new PlayerBehavior { Username = "Hero", PasswordHash = passwordHash }; + playerBehavior.Ban(); + player.Behaviors.Add(playerBehavior); + room.Add(player); + world.Register(player); + + var (loginFlow, _) = MakeLoginFlow(world, room); + var session = Substitute.For(); + session.IsConnected.Returns(true, false); + session.ReadLineAsync(Arg.Any()).Returns("Hero", "correct-horse"); + + var result = await loginFlow.RunAsync(session, TestContext.Current.CancellationToken); + + result.Should().BeNull(); + await session.Received(1).WriteLineAsync("This account has been banned.", Arg.Any()); + } + + [Fact] + public async Task RunAsync_GrantsFullAdmin_WhenUsernameMatchesInitialAdmin() + { + var world = new World(); + var room = MakeRoom(); + world.Register(room); + + var passwordHash = PasswordHashing.Hash("correct-horse"); + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + var playerBehavior = new PlayerBehavior { Username = "Hero", PasswordHash = passwordHash }; + player.Behaviors.Add(playerBehavior); + room.Add(player); + world.Register(player); + + var (loginFlow, repository) = MakeLoginFlow(world, room, initialAdminUsername: "hero"); + var session = Substitute.For(); + session.IsConnected.Returns(true); + session.ReadLineAsync(Arg.Any()).Returns("Hero", "correct-horse"); + + var result = await loginFlow.RunAsync(session, TestContext.Current.CancellationToken); + + result.Should().Be(player); + playerBehavior.Roles.Should().HaveFlag(SecurityRole.FullAdmin); + await repository.Received(1).SaveTreeAsync(player, Arg.Any()); + } + + [Fact] + public async Task RunAsync_GrantsNothing_WhenUsernameDoesNotMatchInitialAdmin() + { + var world = new World(); + var room = MakeRoom(); + world.Register(room); + + var passwordHash = PasswordHashing.Hash("correct-horse"); + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + var playerBehavior = new PlayerBehavior { Username = "Hero", PasswordHash = passwordHash }; + player.Behaviors.Add(playerBehavior); + room.Add(player); + world.Register(player); + + var (loginFlow, _) = MakeLoginFlow(world, room, initialAdminUsername: "SomeoneElse"); + var session = Substitute.For(); + session.IsConnected.Returns(true); + session.ReadLineAsync(Arg.Any()).Returns("Hero", "correct-horse"); + + var result = await loginFlow.RunAsync(session, TestContext.Current.CancellationToken); + + result.Should().Be(player); + playerBehavior.Roles.Should().Be(SecurityRole.Player); + } + + [Fact] + public async Task RunAsync_GrantsNothing_WhenInitialAdminUsernameIsNull() + { + var world = new World(); + var room = MakeRoom(); + world.Register(room); + + var passwordHash = PasswordHashing.Hash("correct-horse"); + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + var playerBehavior = new PlayerBehavior { Username = "Hero", PasswordHash = passwordHash }; + player.Behaviors.Add(playerBehavior); + room.Add(player); + world.Register(player); + + var (loginFlow, repository) = MakeLoginFlow(world, room); + var session = Substitute.For(); + session.IsConnected.Returns(true); + session.ReadLineAsync(Arg.Any()).Returns("Hero", "correct-horse"); + + var result = await loginFlow.RunAsync(session, TestContext.Current.CancellationToken); + + result.Should().Be(player); + playerBehavior.Roles.Should().Be(SecurityRole.Player); + await repository.DidNotReceive().SaveTreeAsync(Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task RunAsync_DoesNotReSave_WhenInitialAdminAlreadyHoldsFullAdmin() + { + var world = new World(); + var room = MakeRoom(); + world.Register(room); + + var passwordHash = PasswordHashing.Hash("correct-horse"); + var player = new Thing { Id = ThingId.New(), Name = "Hero" }; + var playerBehavior = new PlayerBehavior { Username = "Hero", PasswordHash = passwordHash }; + playerBehavior.GrantRole(SecurityRole.FullAdmin); + player.Behaviors.Add(playerBehavior); + room.Add(player); + world.Register(player); + + var (loginFlow, repository) = MakeLoginFlow(world, room, initialAdminUsername: "Hero"); + var session = Substitute.For(); + session.IsConnected.Returns(true); + session.ReadLineAsync(Arg.Any()).Returns("Hero", "correct-horse"); + + var result = await loginFlow.RunAsync(session, TestContext.Current.CancellationToken); + + result.Should().Be(player); + await repository.DidNotReceive().SaveTreeAsync(Arg.Any(), Arg.Any()); + } } diff --git a/tests/SharpMud.Hosting.Tests/SessionLoopTests.cs b/tests/SharpMud.Hosting.Tests/SessionLoopTests.cs index ee770df..3c8d5c5 100644 --- a/tests/SharpMud.Hosting.Tests/SessionLoopTests.cs +++ b/tests/SharpMud.Hosting.Tests/SessionLoopTests.cs @@ -134,6 +134,33 @@ public async Task RunAsync_SavesWithParentStillSet_WhenPlayerQuits() parentAtSaveTime.Should().Be(room); } + [Fact] + public async Task RunAsync_RemovesImmediately_WhenPlayerWasBooted() + { + // ADR-0005: an admin's BootCommand/BanCommand sets WasBooted on a + // different call stack before disconnecting this session - this + // player's own SessionLoop must treat that exactly like an + // explicit quit (immediate removal), not Linkdead (which would let + // the player just reconnect and undo the boot/ban). + var (worldContext, world, room, player, playerBehavior) = MakePlayerInRoom(); + var session = Substitute.For(); + session.IsConnected.Returns(true); + session.ReadLineAsync(Arg.Any()).Returns((string?)null); + playerBehavior.Session = session; + playerBehavior.MarkBooted(); + + var parser = Substitute.For(); + var registry = Substitute.For(); + var repository = Substitute.For(); + var sessionLoop = new SessionLoop(worldContext, parser, registry, repository); + + await sessionLoop.RunAsync(session, player, TestContext.Current.CancellationToken); + + room.Children.Should().NotContain(player); + world.GetThing(player.Id).Should().BeNull(); + playerBehavior.ConnectionState.Should().Be(ConnectionState.Playing); + } + private sealed class StubQuitCommand : ICommand { public string Verb => "quit"; diff --git a/tests/SharpMud.Hosting.Tests/SharpMudHostOptionsTests.cs b/tests/SharpMud.Hosting.Tests/SharpMudHostOptionsTests.cs index 8c2fa44..19a6342 100644 --- a/tests/SharpMud.Hosting.Tests/SharpMudHostOptionsTests.cs +++ b/tests/SharpMud.Hosting.Tests/SharpMudHostOptionsTests.cs @@ -21,4 +21,22 @@ public void Parse_UsesDbPathFromEnvVar_WhenSet() options.DbPath.Should().Be("/data/sharpmud.db"); } + + [Fact] + public void Parse_DefaultsInitialAdminUsernameToNull_WhenEnvVarNotSet() + { + var options = SharpMudHostOptions.Parse(new Dictionary()); + + options.InitialAdminUsername.Should().BeNull(); + } + + [Fact] + public void Parse_UsesInitialAdminUsernameFromEnvVar_WhenSet() + { + var env = new Dictionary { ["SHARPMUD_INITIAL_ADMIN"] = "Root" }; + + var options = SharpMudHostOptions.Parse(env); + + options.InitialAdminUsername.Should().Be("Root"); + } } From 016e1bc449c579e43580a4c3d4f1b869bd8bbe80 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 14:01:09 -0400 Subject: [PATCH 07/10] docs: reflect implemented moderation/admin tooling and ban enforcement Updates SPEC.md, commands.md, accounts-auth.md, deployment.md (SHARPMUD_INITIAL_ADMIN), and plan/roadmap status trackers now that ADR-0005/PLAN-0005's core mechanism is implemented and tested. --- SPEC.md | 28 +++++++++++++------ docs/accounts-auth.md | 14 ++++------ docs/commands.md | 9 +++--- docs/deployment.md | 7 +++++ .../0001-wheelmud-reconciliation-roadmap.md | 2 +- docs/plans/README.md | 2 +- 6 files changed, 39 insertions(+), 23 deletions(-) diff --git a/SPEC.md b/SPEC.md index 8fad65b..e2cac8a 100644 --- a/SPEC.md +++ b/SPEC.md @@ -156,8 +156,9 @@ derivatives separate "instant" commands from round-based combat resolution. 7. **Accounts & auth**: see below. New Telnet connections currently prompt for a plain name (no identity verification) as a placeholder until this phase lands. -8. **Moderation/admin tooling**, **procedural frontier generation**, - **in-game building/scripting**: later phases (see Deferred/Open Items). +8. **Moderation/admin tooling**: implemented — see below. +9. **Procedural frontier generation**, **in-game building/scripting**: later + phases (see Deferred/Open Items). ## Accounts & Auth ✅ implemented @@ -169,6 +170,20 @@ login prompt, one character per login (no separate Account entity, no placeholder name-only prompt has been replaced by the real thing; local CLI still has no login at all, per the original decision. +## Moderation & Admin Tooling ✅ implemented + +`SecurityRole` flags enum + `RoleGuardedCommand`/`MuteGuardedCommand` +decorators around `ICommand`, registered via +`ICommandRegistry.RegisterWithRole` (vs. the unguarded `RegisterOpen`). +`boot`/`mute`/`unmute`/`announce`/`ban`/`unban`/`rolegrant`/`rolerevoke` +commands under `src/SharpMud.Engine/Commands/Builtin/Admin/`. Bootstrap: the +`SHARPMUD_INITIAL_ADMIN` env var (see +[docs/deployment.md](docs/deployment.md)) grants the first `FullAdmin` on +login. See [ADR-0005](docs/adr/0005-security-role-model-and-moderation-commands.md) +and [PLAN-0005](docs/plans/0005-security-role-model-and-moderation-commands.md). +Audit logging of moderation actions remains undesigned — tracked as an open +item below. + ## Deployment Containerized (Docker) ✅ — multi-stage `Dockerfile` at the repo root, built @@ -185,13 +200,10 @@ configuration and open items. Explicitly out of scope for v1, to revisit later: -- **Moderation/admin tooling**: permission levels, mute/kick/ban, admin - commands — designed, see - [ADR-0005](docs/adr/0005-security-role-model-and-moderation-commands.md) - and +- **Moderation audit logging**: which actions get logged, retention, and + where — undesigned. Tracked as an open item in [PLAN-0005](docs/plans/0005-security-role-model-and-moderation-commands.md); - not yet implemented. Audit logging remains undesigned, tracked as an - open item in PLAN-0005. + the moderation commands themselves are implemented (see above). - **Soft-code/scripting engine**: revisit once data/config-driven NPC and room behavior proves insufficient. - **Procedural frontier generation algorithm**: choice of generation approach diff --git a/docs/accounts-auth.md b/docs/accounts-auth.md index 7669160..3c715db 100644 --- a/docs/accounts-auth.md +++ b/docs/accounts-auth.md @@ -79,6 +79,10 @@ name-only placeholder): username existed), retry up to 3 attempts (not a considered final policy — see Open Items), then loop back to the username prompt rather than dropping the connection outright. Correct password, and: + - the character's `PlayerBehavior.IsBanned` (ADR-0005) → `"This account + has been banned."`, connection dropped, before any of the checks + below — a banned account never reaches the already-logged-in/Linkdead + branches. - the character is still actively `Playing` with a live, connected session → `"That character is already logged in."`, connection dropped (unchanged by ADR-0004 — see @@ -149,15 +153,9 @@ silently skipped). "non-empty" is enforced. - Password reset flow — with no email/OAuth identity backing the account, there's no "forgot password" recovery path; not designed. Likely - admin-assisted reset only (ties into the moderation tooling designed in - [ADR-0005](adr/0005-security-role-model-and-moderation-commands.md), not - yet implemented). -- Ban enforcement — designed in + admin-assisted reset only (ties into the moderation tooling in [ADR-0005](adr/0005-security-role-model-and-moderation-commands.md)/ - [PLAN-0005](plans/0005-security-role-model-and-moderation-commands.md) - (a banned `PlayerBehavior.IsBanned` rejects login at password - verification), not yet implemented — today, login has no concept of a - banned user at all. + [PLAN-0005](plans/0005-security-role-model-and-moderation-commands.md)). - The password itself travels in cleartext over Telnet (only the on-screen *display* is suppressed) — no transport encryption exists yet; SSH (see [networking.md](networking.md)) would be the natural place this gets diff --git a/docs/commands.md b/docs/commands.md index fd123f2..08dde87 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -126,12 +126,11 @@ actually implemented as of the inventory/items build-order phase: - **Meta** ✅: `help`, `quit`. - **Builder/OLC verbs** (`@dig`, `@describe`, etc.) explicitly excluded — those belong to the deferred in-game building phase (see - [world-model.md](world-model.md)). **Moderation/admin verbs** + [world-model.md](world-model.md)). **Moderation/admin verbs** ✅ (`boot`/`mute`/`unmute`/`announce`/`ban`/`unban`/`rolegrant`/ - `rolerevoke`) are a separate thing — designed in - [ADR-0005](adr/0005-security-role-model-and-moderation-commands.md), not - yet implemented (see - [PLAN-0005](plans/0005-security-role-model-and-moderation-commands.md)). + `rolerevoke`) are role-gated via `ICommandRegistry.RegisterWithRole` — + see [ADR-0005](adr/0005-security-role-model-and-moderation-commands.md) + and [PLAN-0005](plans/0005-security-role-model-and-moderation-commands.md). ## Open Items diff --git a/docs/deployment.md b/docs/deployment.md index 3e22ac0..a156e17 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -37,6 +37,13 @@ debugging inside the container) without rebuilding. | Mode | `--telnet` (anywhere in args) | `SHARPMUD_MODE=telnet` | CLI (local single-player) | | Telnet port | `--telnet ` (looked up by index, not positional - can combine with `--db-path` in either order) | `SHARPMUD_TELNET_PORT` | `4000` | | SQLite DB path | `--db-path ` | `SHARPMUD_DB_PATH` | `./sharpmud.db` (`/data/sharpmud.db` in the container image) | +| Initial admin username | *(none)* | `SHARPMUD_INITIAL_ADMIN` | unset (no bootstrap grant) | + +`SHARPMUD_INITIAL_ADMIN` (ADR-0005) names the one character `LoginFlow` +grants `SecurityRole.FullAdmin` to on login — the only path to a `FullAdmin` +on a fresh deployment, since granting a role itself requires `FullAdmin`. +Idempotent (checked every login, no-op once already held), so it's safe to +leave set permanently rather than unsetting it after first use. The Dockerfile sets `SHARPMUD_MODE=telnet`, `SHARPMUD_TELNET_PORT=4000`, and `SHARPMUD_DB_PATH=/data/sharpmud.db` as image defaults, plus `EXPOSE 4000` diff --git a/docs/plans/0001-wheelmud-reconciliation-roadmap.md b/docs/plans/0001-wheelmud-reconciliation-roadmap.md index e738f8d..39f4c2d 100644 --- a/docs/plans/0001-wheelmud-reconciliation-roadmap.md +++ b/docs/plans/0001-wheelmud-reconciliation-roadmap.md @@ -29,7 +29,7 @@ reconciliation effort stands. Accepted, PLAN-0004 Done. See [PLAN-0004](0004-session-state-machine-and-reconnect.md). - [ ] **Slice 3 — Permission/security-role model + moderation commands.** - ADR-0005 Accepted, PLAN-0005 Not Started. See + ADR-0005 Accepted, PLAN-0005 In Progress. See [PLAN-0005](0005-security-role-model-and-moderation-commands.md). - [ ] **Slice 4 — World-building/OLC command surface.** Not yet designed; bundles with Slice 3. diff --git a/docs/plans/README.md b/docs/plans/README.md index f7bae95..caca1df 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -59,6 +59,6 @@ isn't settled yet — don't do that. | [0001](0001-wheelmud-reconciliation-roadmap.md) | [ADR-0001](../adr/0001-wheelmud-reconciliation-roadmap.md) | In Progress | | [0002](0002-telnet-protocol-negotiation.md) | [ADR-0002](../adr/0002-telnet-protocol-negotiation.md) | Done | | [0004](0004-session-state-machine-and-reconnect.md) | [ADR-0004](../adr/0004-session-state-machine-and-reconnect.md) | Done | -| [0005](0005-security-role-model-and-moderation-commands.md) | [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) | Not Started | +| [0005](0005-security-role-model-and-moderation-commands.md) | [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) | In Progress | | [0006](0006-nuget-package-distribution.md) | [ADR-0006](../adr/0006-nuget-package-distribution.md) | Done | | [0008](0008-ruleset-scaffolding-tier.md) | [ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) | Done | From 6b67de1b9d2945cdde5347380ed7cab5c998efb5 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 14:08:12 -0400 Subject: [PATCH 08/10] docs: close out PLAN-0005 after manual Telnet verification All 9 verification steps (bootstrap, restart idempotency, rolegrant, mute/unmute, ban/unban, boot+reconnect regression check, announce, non-admin rejection, restart persistence) passed manually. Flips PLAN-0005/roadmap Slice 3 to Done; task checklist fully checked. Noted a pre-existing, unrelated bug found during verification: the game loop's WanderManager broadcast can crash the process writing to a Linkdead session's dead socket - not part of ADR-0005's scope. --- .../0001-wheelmud-reconciliation-roadmap.md | 4 +- ...rity-role-model-and-moderation-commands.md | 76 ++++++++++--------- docs/plans/README.md | 2 +- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/docs/plans/0001-wheelmud-reconciliation-roadmap.md b/docs/plans/0001-wheelmud-reconciliation-roadmap.md index 39f4c2d..e322766 100644 --- a/docs/plans/0001-wheelmud-reconciliation-roadmap.md +++ b/docs/plans/0001-wheelmud-reconciliation-roadmap.md @@ -28,8 +28,8 @@ reconciliation effort stands. - [x] **Slice 2 — Session/connection state machine + reconnect.** ADR-0004 Accepted, PLAN-0004 Done. See [PLAN-0004](0004-session-state-machine-and-reconnect.md). -- [ ] **Slice 3 — Permission/security-role model + moderation commands.** - ADR-0005 Accepted, PLAN-0005 In Progress. See +- [x] **Slice 3 — Permission/security-role model + moderation commands.** + ADR-0005 Accepted, PLAN-0005 Done. See [PLAN-0005](0005-security-role-model-and-moderation-commands.md). - [ ] **Slice 4 — World-building/OLC command surface.** Not yet designed; bundles with Slice 3. diff --git a/docs/plans/0005-security-role-model-and-moderation-commands.md b/docs/plans/0005-security-role-model-and-moderation-commands.md index 43b0850..27221ce 100644 --- a/docs/plans/0005-security-role-model-and-moderation-commands.md +++ b/docs/plans/0005-security-role-model-and-moderation-commands.md @@ -2,7 +2,7 @@ **Implements:** [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) -**Status:** In Progress +**Status:** Done **Last updated:** 2026-07-23 @@ -57,7 +57,7 @@ an open item below. ### `SecurityRole` + registry -- [ ] New `src/SharpMud.Engine/Commands/SecurityRole.cs` — plain +- [x] New `src/SharpMud.Engine/Commands/SecurityRole.cs` — plain `[Flags] enum SecurityRole : uint` with **explicit power-of-two values on every member** (`None = 0`, `Mobile = 1 << 0`, `Item = 1 << 1`, `Room = 1 << 2`, `TutorialPlayer = 1 << 3`, `Player = 1 << @@ -73,24 +73,24 @@ an open item below. `All` is a union expression, not a separate hardcoded value, so it can't drift out of sync if a flag is added later. XML doc comments per `documentation.md`'s new-public-member rule. -- [ ] New `src/SharpMud.Engine/Commands/RoleGuardedCommand.cs` — wraps an +- [x] New `src/SharpMud.Engine/Commands/RoleGuardedCommand.cs` — wraps an inner `ICommand`, checks `(actor.Roles & requiredRole) != SecurityRole.None` before delegating; generic rejection message. Exposes `public SecurityRole RequiredRole { get; }` — needed by `HelpCommand`'s filtering below, not just internally. -- [ ] New `src/SharpMud.Engine/Commands/MuteGuardedCommand.cs` — wraps an +- [x] New `src/SharpMud.Engine/Commands/MuteGuardedCommand.cs` — wraps an inner `ICommand`, checks the *actor's own* `IsMuted` (not a target's — this gates the muted player's own `say`/`emote`, not something they're doing to someone else) before delegating. -- [ ] `ICommandRegistry.cs` / `CommandRegistry.cs` +- [x] `ICommandRegistry.cs` / `CommandRegistry.cs` (`src/SharpMud.Engine/Commands/`): remove `Register(ICommand)` from the public surface; add `RegisterOpen(ICommand)` and `RegisterWithRole(ICommand, SecurityRole)` (the latter wraps in `RoleGuardedCommand` before storing). -- [ ] Update every existing registration call site from `Register` to +- [x] Update every existing registration call site from `Register` to `RegisterOpen` — mechanical, no behavior change. Two call sites today, not one (verified by grep, not assumed): - - [ ] `src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs` — all + - [x] `src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs` — all 17 registrations. While here: wrap the `SayCommand`/ `EmoteCommand` registrations specifically in `MuteGuardedCommand` before `RegisterOpen` (mute enforcement is @@ -98,19 +98,19 @@ an open item below. .RegisterAll` runs for every consumer via `Hosting`'s `AddSharpMudRuleset(...)`, so wrapping here — not per-consumer — is what actually makes mute universal). - - [ ] `src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs` — the + - [x] `src/SharpMud.Ruleset.Rpg/ServiceCollectionExtensions.cs` — the `AttackCommand`/`FleeCommand` registrations inside `AddSharpMudRpgRuleset(...)`. This call site didn't exist when ADR-0005 was accepted ([ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) added it afterward) — `attack`/`flee` aren't role-gated, just migrated to the new intentional entry point like everything else. - - [ ] `samples/SharpMud.Samples.Classic/ClassicCommands.cs` no longer + - [x] `samples/SharpMud.Samples.Classic/ClassicCommands.cs` no longer exists (removed when [ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) landed — it had nothing left to register) — do not recreate it for this plan; the new admin commands register through their own `AdminCommands.RegisterAll`, see below. -- [ ] `HelpCommand.cs`: filter `registry.Commands` by the actor's roles +- [x] `HelpCommand.cs`: filter `registry.Commands` by the actor's roles before listing them — caught in self-review. `RoleGuardedCommand` passes `Verb`/`Aliases` straight through from the wrapped command, so without this, `help` lists every admin command (`ban`, `boot`, @@ -122,13 +122,13 @@ an open item below. ### `PlayerBehavior` + persistence + `SessionLoop` -- [ ] `src/SharpMud.Engine/Behaviors/PlayerBehavior.cs`: add `SecurityRole +- [x] `src/SharpMud.Engine/Behaviors/PlayerBehavior.cs`: add `SecurityRole Roles { get; private set; } = SecurityRole.Player`, `bool IsMuted { get; private set; }`, `bool IsBanned { get; private set; }`, plus mutation methods (`GrantRole`/`RevokeRole`, `Mute`/`Unmute`, `Ban`/`Unban`) rather than public setters, matching `ConnectionState`'s existing transition-method style. - - [ ] Also add `bool WasBooted { get; private set; }` (transient, + - [x] Also add `bool WasBooted { get; private set; }` (transient, like `Session`/`ConnectionState` — `Ignore`d in `PlayerBehaviorConfiguration`) and a `MarkBooted()` method. **Real gap caught in PR review**: `BootCommand` runs inside @@ -145,7 +145,7 @@ an open item below. `PlayerBehavior` *before* calling `DisconnectAsync`; the target's own `SessionLoop.RunAsync` (a completely separate call stack) checks it in its `finally` block. - - [ ] `src/SharpMud.Hosting/SessionLoop.cs`: in the `finally` block, + - [x] `src/SharpMud.Hosting/SessionLoop.cs`: in the `finally` block, treat `WasBooted` exactly like `explicitQuit` — both mean "this disconnect was intentional, skip `Linkdead` and remove immediately" (same save-then-remove ordering `explicitQuit` @@ -153,19 +153,19 @@ an open item below. ordering). Concretely: replace the bare `explicitQuit` checks in both branches with `explicitQuit || (playerBehavior ?.WasBooted ?? false)`. - - [ ] `GrantRole(SecurityRole role)`: ORs in `role` *and* every tier + - [x] `GrantRole(SecurityRole role)`: ORs in `role` *and* every tier it implies (`FullAdmin` → also `MinorAdmin` + `Player`; `FullBuilder` → also `MinorBuilder`) per ADR-0005's accumulation rule — a plain `Roles |= role` is not enough on its own. - - [ ] A static `SecurityRole.Implies(SecurityRole role)` (or + - [x] A static `SecurityRole.Implies(SecurityRole role)` (or equivalent lookup) expressing the same hierarchy (`FullAdmin`→`MinorAdmin`→`Player`, `FullBuilder`→ `MinorBuilder`) — used by both `GrantRole` (to accumulate downward) and `RevokeRole` (to check upward, see next) so the hierarchy is defined once, not duplicated between the two directions. - - [ ] `RevokeRole(SecurityRole role)`: **enforces the same + - [x] `RevokeRole(SecurityRole role)`: **enforces the same invariant symmetrically** (caught in PR review — clearing only the exact bit passed in can leave a higher tier set with a lower tier it implies cleared, e.g. `FullAdmin` present but @@ -182,7 +182,7 @@ an open item below. has FullAdmin, which includes MinorAdmin — revoke FullAdmin instead"). `RoleRevokeCommand` relays a non-null return straight to the admin; never wraps this call in a try/catch. -- [ ] `src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs`: +- [x] `src/SharpMud.Persistence/Configurations/PlayerBehaviorConfiguration.cs`: map `Roles` with the plain-enum default EF conversion (matching `WearableBehaviorConfiguration`'s `Slot` precedent — no custom value converter needed); map `IsMuted`/`IsBanned` as plain persisted @@ -236,7 +236,7 @@ playerBehavior.Session is { IsConnected: true }`) — don't invent a narrower one. Verified this check still reads exactly this way in the current `src/SharpMud.Hosting/LoginFlow.cs`. -- [ ] `BootCommand` (`MinorAdmin`, no repository dependency) — disconnects +- [x] `BootCommand` (`MinorAdmin`, no repository dependency) — disconnects a currently-online (`ConnectionState == Playing && Session is { IsConnected: true }`) target by username; "not online" message otherwise (not found at all, found only `Linkdead`, or found but @@ -247,15 +247,15 @@ current `src/SharpMud.Hosting/LoginFlow.cs`. a message to the target's session first (mirrors `QuitCommand`'s "Goodbye!" before disconnecting), e.g. "You have been disconnected by an administrator." -- [ ] `MuteCommand`/`UnmuteCommand` (`MinorAdmin`, `IThingRepository`) — +- [x] `MuteCommand`/`UnmuteCommand` (`MinorAdmin`, `IThingRepository`) — sets/clears `IsMuted` on a target (online-or-not, mirrors `LoginFlow`'s live-then-repository lookup), saves immediately. -- [ ] `AnnounceCommand` (`MinorAdmin`, no repository dependency) — +- [x] `AnnounceCommand` (`MinorAdmin`, no repository dependency) — broadcasts to every `world.AllWithBehavior()` entry whose `ConnectionState == Playing && Session is { IsConnected: true }` — explicitly **not** every entry `WhoCommand`-style, and explicitly **not** `ConnectionState` alone (see above). -- [ ] `BanCommand` (`FullAdmin`, `IThingRepository`) — sets `IsBanned`, +- [x] `BanCommand` (`FullAdmin`, `IThingRepository`) — sets `IsBanned`, online-or-not lookup, saves immediately. **If the target is currently online (`ConnectionState == Playing && Session is { IsConnected: true }`), also disconnects them the same way @@ -270,7 +270,7 @@ current `src/SharpMud.Hosting/LoginFlow.cs`. alone, both are harmless and trivially reversible by the same admin). `UnbanCommand` needs no such guard (undoing your own ban isn't reachable — you can't be logged in while banned). -- [ ] `RoleGrantCommand`/`RoleRevokeCommand` (`FullAdmin`, +- [x] `RoleGrantCommand`/`RoleRevokeCommand` (`FullAdmin`, `IThingRepository`) — mutates a target's `Roles` via `GrantRole`/`RevokeRole` (accumulation/hierarchy-invariant enforcement happens inside `PlayerBehavior` itself, not here), @@ -290,7 +290,7 @@ current `src/SharpMud.Hosting/LoginFlow.cs`. checks `RevokeRole`'s `string?` return (not a caught exception) and relays a non-null failure straight to the admin as the rejection message. -- [ ] Register all 8 via `RegisterWithRole` in a new +- [x] Register all 8 via `RegisterWithRole` in a new `AdminCommands.RegisterAll(registry, repository)` (`src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommands.cs` — mirrors `BuiltinCommands`'s shape). **Wiring point, corrected from @@ -325,13 +325,13 @@ character is brand-new (`MaybeCreateAsync`) or pre-existing and "admin logs in later after the env var was set," all identically — no separate boot-time code path to keep in sync with the login-time one. -- [ ] `LoginFlow.LoginExistingAsync`: after password verification +- [x] `LoginFlow.LoginExistingAsync`: after password verification succeeds, check `IsBanned` → reject with a distinct message before the `ConnectionState` branch. -- [ ] `src/SharpMud.Hosting/SharpMudHostOptions.cs`: add `string? +- [x] `src/SharpMud.Hosting/SharpMudHostOptions.cs`: add `string? InitialAdminUsername`, parsed from `SHARPMUD_INITIAL_ADMIN` in `SharpMudHostOptions.Parse(env)`. -- [ ] `samples/SharpMud.Samples.Classic/Program.cs`: add +- [x] `samples/SharpMud.Samples.Classic/Program.cs`: add `["SHARPMUD_INITIAL_ADMIN"] = Environment.GetEnvironmentVariable ("SHARPMUD_INITIAL_ADMIN")` to the `env` dictionary already built before `SharpMudHostOptions.Parse(env)` — that dictionary is a @@ -339,7 +339,7 @@ no separate boot-time code path to keep in sync with the login-time one. `SHARPMUD_DB_PATH`), not a full environment pass-through, so the new var needs its own entry here or the real host never sees it even with it set in production. -- [ ] `builder.Services.AddSingleton(hostOptions);` in the same +- [x] `builder.Services.AddSingleton(hostOptions);` in the same `Program.cs`, right after `SharpMudHostOptions.Parse(env)` — **new requirement this plan's implementer needs that the original didn't**: `SharpMudHostOptions` isn't registered in DI today (`Program.cs` @@ -347,7 +347,7 @@ no separate boot-time code path to keep in sync with the login-time one. directly into `AddSharpMudSqlitePersistence(...)`), but `LoginFlow` needs to constructor-inject it now, so it has to actually be in the container. -- [ ] `LoginFlow`: constructor-inject `SharpMudHostOptions` (a 4th +- [x] `LoginFlow`: constructor-inject `SharpMudHostOptions` (a 4th dependency, alongside `WorldContext`/`IThingRepository`/ `IPlayerFactory`). In `RunAsync`, after `LoginExistingAsync`/ `MaybeCreateAsync` produces a non-null `player`, before returning @@ -363,23 +363,23 @@ no separate boot-time code path to keep in sync with the login-time one. ### Docs -- [ ] `docs/commands.md`: describe the new admin command set as current +- [x] `docs/commands.md`: describe the new admin command set as current state, link ADR-0005. -- [ ] `docs/accounts-auth.md`: describe ban enforcement in the login flow +- [x] `docs/accounts-auth.md`: describe ban enforcement in the login flow as current state, update the existing "deferred moderation tooling" forward-reference to point at ADR-0005/this plan. -- [ ] `docs/deployment.md`: add `SHARPMUD_INITIAL_ADMIN` to the Runtime +- [x] `docs/deployment.md`: add `SHARPMUD_INITIAL_ADMIN` to the Runtime Configuration table — this table is the documented list of every `SharpMudHostOptions`/sample-level env var, and `SHARPMUD_INITIAL_ADMIN` is the *only* bootstrap path to a `FullAdmin` in a fresh deployment; omitting it here means deploying a container with no discoverable way to administer it. -- [ ] `SPEC.md`: update the "Moderation/admin tooling" Deferred/Open Item +- [x] `SPEC.md`: update the "Moderation/admin tooling" Deferred/Open Item to reflect what's actually implemented vs. still deferred (Find/ GoTo/Control/Clone/Spawn/Jail/Buff/Relinquish, audit logging). -- [ ] `docs/adr/README.md` / `docs/plans/README.md`: index rows for +- [x] `docs/adr/README.md` / `docs/plans/README.md`: index rows for ADR-0005/PLAN-0005 flip to `Accepted`/in-progress-then-`Done`. -- [ ] `docs/plans/0001-wheelmud-reconciliation-roadmap.md`: check off +- [x] `docs/plans/0001-wheelmud-reconciliation-roadmap.md`: check off Slice 3. ## Critical files @@ -491,6 +491,14 @@ Modified: ## Verification +**Done (2026-07-23)** — all 9 steps below run manually over real Telnet +connections against the Classic sample; all passed as described. Found and +noted (not fixed, out of scope) one pre-existing, unrelated crash: the +game loop's `WanderManager` tick broadcasts to every occupant of a room +including `Linkdead` ones with a dead socket, and an unhandled +`IOException`/`SocketException` there brings down the whole process — this +predates ADR-0005 and isn't part of its mechanism. + Real manual check over Telnet (this repo's established pattern for session/persistence-facing changes): diff --git a/docs/plans/README.md b/docs/plans/README.md index caca1df..76bf7da 100644 --- a/docs/plans/README.md +++ b/docs/plans/README.md @@ -59,6 +59,6 @@ isn't settled yet — don't do that. | [0001](0001-wheelmud-reconciliation-roadmap.md) | [ADR-0001](../adr/0001-wheelmud-reconciliation-roadmap.md) | In Progress | | [0002](0002-telnet-protocol-negotiation.md) | [ADR-0002](../adr/0002-telnet-protocol-negotiation.md) | Done | | [0004](0004-session-state-machine-and-reconnect.md) | [ADR-0004](../adr/0004-session-state-machine-and-reconnect.md) | Done | -| [0005](0005-security-role-model-and-moderation-commands.md) | [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) | In Progress | +| [0005](0005-security-role-model-and-moderation-commands.md) | [ADR-0005](../adr/0005-security-role-model-and-moderation-commands.md) | Done | | [0006](0006-nuget-package-distribution.md) | [ADR-0006](../adr/0006-nuget-package-distribution.md) | Done | | [0008](0008-ruleset-scaffolding-tier.md) | [ADR-0008](../adr/0008-ruleset-scaffolding-tier.md) | Done | From 527b00c3f358bfa5870bd8c1a4b50ee98458ffc3 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 14:26:44 -0400 Subject: [PATCH 09/10] fix: builder roles imply Player, mirroring the admin ladder FullBuilder/MinorBuilder now expand to include Player in ImpliedRoles, same as MinorAdmin does - a builder should always behave as an ordinary player too. Admin and builder ladders stay independent (FullAdmin still does not imply FullBuilder, by design). RevokeRole's hierarchy guard now also blocks revoking Player out from under a held builder role. --- .../Behaviors/PlayerBehavior.cs | 4 +- .../Commands/SecurityRoleExtensions.cs | 13 +++--- .../Behaviors/PlayerBehaviorSecurityTests.cs | 41 ++++++++++++++++++- .../Commands/SecurityRoleTests.cs | 25 ++++++++++- 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs b/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs index 3e97e56..d5753bb 100644 --- a/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs +++ b/src/SharpMud.Engine/Behaviors/PlayerBehavior.cs @@ -146,7 +146,7 @@ public void Reconnect() // The only roles with an "implies" relationship to another role - see // SecurityRoleExtensions.ImpliedRoles. RevokeRole only needs to check - // these three, not every SecurityRole member. + // these four, not every SecurityRole member. private static readonly SecurityRole[] RolesWithImplications = - [SecurityRole.FullAdmin, SecurityRole.MinorAdmin, SecurityRole.FullBuilder]; + [SecurityRole.FullAdmin, SecurityRole.MinorAdmin, SecurityRole.FullBuilder, SecurityRole.MinorBuilder]; } diff --git a/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs b/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs index c73a316..1c32682 100644 --- a/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs +++ b/src/SharpMud.Engine/Commands/SecurityRoleExtensions.cs @@ -4,10 +4,12 @@ namespace SharpMud.Engine.Commands; /// The role hierarchy ( implies implies ; /// implies ), defined once here and used by both -/// PlayerBehavior.GrantRole (accumulate downward) and -/// PlayerBehavior.RevokeRole (check upward) per ADR-0005's -/// accumulation rule. +/// cref="SecurityRole.MinorBuilder"/> implies +/// - the admin and builder ladders are deliberately independent of each +/// other, both bottoming out at ), defined +/// once here and used by both PlayerBehavior.GrantRole (accumulate +/// downward) and PlayerBehavior.RevokeRole (check upward) per +/// ADR-0005's accumulation rule. /// public static class SecurityRoleExtensions { @@ -23,7 +25,8 @@ public static class SecurityRoleExtensions { SecurityRole.FullAdmin => SecurityRole.FullAdmin | SecurityRole.MinorAdmin | SecurityRole.Player, SecurityRole.MinorAdmin => SecurityRole.MinorAdmin | SecurityRole.Player, - SecurityRole.FullBuilder => SecurityRole.FullBuilder | SecurityRole.MinorBuilder, + SecurityRole.FullBuilder => SecurityRole.FullBuilder | SecurityRole.MinorBuilder | SecurityRole.Player, + SecurityRole.MinorBuilder => SecurityRole.MinorBuilder | SecurityRole.Player, _ => role, }; diff --git a/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs b/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs index 707a4cd..2bd652d 100644 --- a/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs +++ b/tests/SharpMud.Engine.Tests/Behaviors/PlayerBehaviorSecurityTests.cs @@ -28,7 +28,7 @@ public void GrantRole_FullAdmin_AlsoGrantsMinorAdminAndPlayer() } [Fact] - public void GrantRole_FullBuilder_AlsoGrantsMinorBuilder() + public void GrantRole_FullBuilder_AlsoGrantsMinorBuilderAndPlayer() { var player = MakePlayer(); @@ -36,6 +36,18 @@ public void GrantRole_FullBuilder_AlsoGrantsMinorBuilder() player.Roles.Should().HaveFlag(SecurityRole.FullBuilder); player.Roles.Should().HaveFlag(SecurityRole.MinorBuilder); + player.Roles.Should().HaveFlag(SecurityRole.Player); + } + + [Fact] + public void GrantRole_FullBuilder_DoesNotGrantAnyAdminRole() + { + var player = MakePlayer(); + + player.GrantRole(SecurityRole.FullBuilder); + + player.Roles.Should().NotHaveFlag(SecurityRole.MinorAdmin); + player.Roles.Should().NotHaveFlag(SecurityRole.FullAdmin); } [Fact] @@ -89,6 +101,33 @@ public void RevokeRole_MinorAdmin_Succeeds_WhenActorDoesNotAlsoHoldFullAdmin() player.Roles.Should().NotHaveFlag(SecurityRole.MinorAdmin); } + [Fact] + public void RevokeRole_Player_ReturnsFailureAndLeavesRolesUnchanged_WhenActorAlsoHoldsFullBuilder() + { + var player = MakePlayer(); + player.GrantRole(SecurityRole.FullBuilder); + var rolesBefore = player.Roles; + + var result = player.RevokeRole(SecurityRole.Player); + + result.Should().NotBeNull(); + player.Roles.Should().Be(rolesBefore); + } + + [Fact] + public void RevokeRole_FullBuilder_SucceedsAndLeavesMinorBuilderAndPlayerIntact_WhenActorHoldsFullBuilder() + { + var player = MakePlayer(); + player.GrantRole(SecurityRole.FullBuilder); + + var result = player.RevokeRole(SecurityRole.FullBuilder); + + result.Should().BeNull(); + player.Roles.Should().NotHaveFlag(SecurityRole.FullBuilder); + player.Roles.Should().HaveFlag(SecurityRole.MinorBuilder); + player.Roles.Should().HaveFlag(SecurityRole.Player); + } + [Fact] public void Mute_SetsIsMuted_AndUnmute_ClearsIt() { diff --git a/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs b/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs index ebd86ef..f6ebda0 100644 --- a/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/SecurityRoleTests.cs @@ -58,12 +58,33 @@ public void ImpliedRoles_ForFullAdmin_IncludesMinorAdminAndPlayer() } [Fact] - public void ImpliedRoles_ForFullBuilder_IncludesMinorBuilder() + public void ImpliedRoles_ForFullBuilder_IncludesMinorBuilderAndPlayer() { var implied = SecurityRole.FullBuilder.ImpliedRoles; implied.Should().HaveFlag(SecurityRole.FullBuilder); implied.Should().HaveFlag(SecurityRole.MinorBuilder); + implied.Should().HaveFlag(SecurityRole.Player); + } + + [Fact] + public void ImpliedRoles_ForMinorBuilder_IncludesPlayer() + { + var implied = SecurityRole.MinorBuilder.ImpliedRoles; + + implied.Should().HaveFlag(SecurityRole.MinorBuilder); + implied.Should().HaveFlag(SecurityRole.Player); + } + + [Fact] + public void ImpliedRoles_ForFullBuilder_DoesNotIncludeAdminRoles() + { + // The admin and builder ladders are deliberately independent - + // FullAdmin does not imply FullBuilder, and vice versa. + var implied = SecurityRole.FullBuilder.ImpliedRoles; + + implied.Should().NotHaveFlag(SecurityRole.MinorAdmin); + implied.Should().NotHaveFlag(SecurityRole.FullAdmin); } [Fact] @@ -77,6 +98,8 @@ public void ImpliedRoles_ForARoleWithNoHierarchy_IsJustItself() [InlineData(SecurityRole.FullAdmin, SecurityRole.Player, true)] [InlineData(SecurityRole.MinorAdmin, SecurityRole.FullAdmin, false)] [InlineData(SecurityRole.FullBuilder, SecurityRole.MinorBuilder, true)] + [InlineData(SecurityRole.FullBuilder, SecurityRole.Player, true)] + [InlineData(SecurityRole.MinorBuilder, SecurityRole.Player, true)] [InlineData(SecurityRole.MinorBuilder, SecurityRole.FullBuilder, false)] public void Implies_ReflectsTheRoleHierarchy(SecurityRole role, SecurityRole other, bool expected) { From 456245fc3c4b375411d473304f79cbb232f14549 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Thu, 23 Jul 2026 14:47:37 -0400 Subject: [PATCH 10/10] fix: address PR #19 review feedback - Fix xUnit1051: use Arg.Any() instead of default in 6 DidNotReceiveWithAnyArgs verification calls across the admin command tests, matching the positive-path assertions in the same files. - Extract AdminCommandHelpers.FindLiveTarget so BootCommand no longer duplicates FindTargetAsync's live-lookup logic. - Add AdminCommandHelpers.GetOnlineBehavior so AnnounceCommand looks up each player's PlayerBehavior once instead of twice per iteration; IsOnline now delegates to it. --- .../Builtin/Admin/AdminCommandHelpers.cs | 33 ++++++++++++++++--- .../Commands/Builtin/Admin/AnnounceCommand.cs | 4 +-- .../Commands/Builtin/Admin/BootCommand.cs | 3 +- .../Commands/Builtin/Admin/BanCommandTests.cs | 2 +- .../Builtin/Admin/BootCommandTests.cs | 2 +- .../Builtin/Admin/MuteCommandTests.cs | 2 +- .../Builtin/Admin/RoleGrantCommandTests.cs | 2 +- .../Builtin/Admin/RoleRevokeCommandTests.cs | 4 +-- 8 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs index 0815257..89ddcc4 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/AdminCommandHelpers.cs @@ -12,24 +12,47 @@ namespace SharpMud.Engine.Commands.Builtin.Admin; // including one sitting in World with no live session at all). internal static class AdminCommandHelpers { + /// + /// Finds a player by username among Things currently live in the world + /// (registered - online, linkdead, or otherwise), regardless of + /// connection state. Shared by (which + /// falls back to the repository for offline players) and + /// BootCommand (which deliberately never falls back - booting an + /// already-offline target doesn't make sense). + /// + public static Thing? FindLiveTarget(IWorld world, string username) => + world.AllWithBehavior() + .FirstOrDefault(p => string.Equals(p.FindBehavior()!.Username, username, StringComparison.OrdinalIgnoreCase)); + /// Finds a player by username, live in the world first, then falling back to the repository (offline). Never attaches an offline result into the world tree. public static async Task FindTargetAsync(IWorld world, IThingRepository repository, string username, CancellationToken ct) { - var live = world.AllWithBehavior() - .FirstOrDefault(p => string.Equals(p.FindBehavior()!.Username, username, StringComparison.OrdinalIgnoreCase)); + var live = FindLiveTarget(world, username); if (live is not null) return live; return await repository.FindPlayerByUsernameAsync(username, ct); } - /// Whether a player Thing is currently online - both and a connected , not either alone. - public static bool IsOnline(Thing player) + /// + /// The player's if they're currently + /// online - both and a + /// connected , not either alone - or + /// otherwise. Callers that also need the + /// behavior/session (e.g. to write to it) should use this instead of + /// plus a separate FindBehavior call. + /// + public static PlayerBehavior? GetOnlineBehavior(Thing player) { var behavior = player.FindBehavior(); - return behavior is { ConnectionState: var state, Session: { IsConnected: true } } && state == ConnectionState.Playing; + return behavior is { ConnectionState: var state, Session: { IsConnected: true } } && state == ConnectionState.Playing + ? behavior + : null; } + /// Whether a player Thing is currently online. See . + public static bool IsOnline(Thing player) => GetOnlineBehavior(player) is not null; + /// /// Parses an individually-grantable role name - rejects (every current and future flag, not a diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs index 0cc6a6f..aa26f20 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/AnnounceCommand.cs @@ -20,10 +20,10 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) // (ADR-0004), which would attempt a write to their stale session. foreach (var player in ctx.World.AllWithBehavior()) { - if (!AdminCommandHelpers.IsOnline(player)) + if (AdminCommandHelpers.GetOnlineBehavior(player) is not { } behavior) continue; - await player.FindBehavior()!.Session!.WriteLineAsync($"[Announcement] {message}", ct); + await behavior.Session!.WriteLineAsync($"[Announcement] {message}", ct); } } } diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs index 1b7af7c..719e3cf 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/BootCommand.cs @@ -14,8 +14,7 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) return; var username = string.Join(' ', ctx.Args); - var target = ctx.World.AllWithBehavior() - .FirstOrDefault(p => string.Equals(p.FindBehavior()!.Username, username, StringComparison.OrdinalIgnoreCase)); + var target = AdminCommandHelpers.FindLiveTarget(ctx.World, username); if (target is null || !AdminCommandHelpers.IsOnline(target)) { diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs index 64bf70e..62a6439 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BanCommandTests.cs @@ -27,7 +27,7 @@ public async Task ExecuteAsync_RejectsSelfTargeting_AndLeavesIsBannedUnchanged() adminBehavior.IsBanned.Should().BeFalse(); await adminSession.Received(1).WriteLineAsync("You cannot ban yourself.", Arg.Any()); - await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); } [Fact] diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs index fc130f0..f799fd2 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/BootCommandTests.cs @@ -59,7 +59,7 @@ public async Task ExecuteAsync_SendsNotOnlineMessage_WhenTargetIsLinkdead() await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); target.FindBehavior()!.WasBooted.Should().BeFalse(); - await targetSession.DidNotReceiveWithAnyArgs().DisconnectAsync(default!, default); + await targetSession.DidNotReceiveWithAnyArgs().DisconnectAsync(default!, Arg.Any()); await adminSession.Received(1).WriteLineAsync("Target is not online.", Arg.Any()); } diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs index c0a2495..3743212 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/MuteCommandTests.cs @@ -74,6 +74,6 @@ public async Task ExecuteAsync_SendsNotFoundMessage_WhenTargetDoesNotExistAnywhe await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); await adminSession.Received(1).WriteLineAsync("No player named Ghost was found.", Arg.Any()); - await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); } } diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs index 82b2276..277468c 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleGrantCommandTests.cs @@ -53,7 +53,7 @@ public async Task ExecuteAsync_RejectsAllAndNone_RegardlessOfCasing(string roleN await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); - await repository.DidNotReceiveWithAnyArgs().FindPlayerByUsernameAsync(default!, default); + await repository.DidNotReceiveWithAnyArgs().FindPlayerByUsernameAsync(default!, Arg.Any()); await adminSession.Received(1).WriteLineAsync($"'{roleName}' isn't a grantable role.", Arg.Any()); } diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs index 2e6e69f..fffd222 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/RoleRevokeCommandTests.cs @@ -28,7 +28,7 @@ public async Task ExecuteAsync_RejectsRevokingOwnFullAdmin() adminBehavior.Roles.Should().HaveFlag(SecurityRole.FullAdmin); await adminSession.Received(1).WriteLineAsync("You cannot revoke your own FullAdmin.", Arg.Any()); - await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); } [Fact] @@ -102,7 +102,7 @@ public async Task ExecuteAsync_RelaysHierarchyFailure_WithoutSaving() await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); targetBehavior.Roles.Should().HaveFlag(SecurityRole.MinorAdmin); - await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, default); + await repository.DidNotReceiveWithAnyArgs().SaveTreeAsync(default!, Arg.Any()); await adminSession.Received(1).WriteLineAsync( Arg.Is(s => s!.Contains("FullAdmin")), Arg.Any()); }