fix(server): own game-account signup, and repair the gate slice 1 broke
`POST /player/shard/account` and its staff twin have answered 500 for every caller since slice 1: the ported controller called `settings.isGameAccountSignupEnabled()`, which is a member of core's settings model and not of `ctx.settings` — three functions, deliberately. The call was `undefined(...)`, the TypeError landed in the catch, and no test reached the branch. The gate now lives on the side that uses it (`utils/gameSignup.js`), which is also where the policy belongs: the setting's own help text names Bridge.cfg and says the shard's SignupMode must agree, and core cannot own a sentence about a UO shard. The admin field moves to this module's Shard page and the derived flag onto `/public/shard/features`, beside the visibility flags the same callers already read. The setting KEY is unchanged. Renaming `game_account_signup` would silently reset every configured instance to `disabled` on upgrade, with players reporting broken signup as the only clue — the same grandfathering as `spawn_atlas_servuo_path` and the seven stream ids. Both regression tests were shown to fail against the bug before it was fixed. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
160
server/test/gameSignup.test.js
Normal file
160
server/test/gameSignup.test.js
Normal file
@@ -0,0 +1,160 @@
|
||||
// ── Game-account signup: the policy, and the crash it was hiding ───────────
|
||||
//
|
||||
// New in slice 3 of the Phase 3 extraction. `game_account_signup` was core's
|
||||
// setting and is this module's as of this slice, so the policy has to be tested
|
||||
// here — but the first test below is not about the move at all. It is about a
|
||||
// defect slice 1 shipped and no test in either repo could see.
|
||||
//
|
||||
// The ported controller called `settings.isGameAccountSignupEnabled()`, which is
|
||||
// a member of core's settings MODEL and not of `ctx.settings` — three functions,
|
||||
// deliberately (MODULE_API.md §2.3). So the call was `undefined(...)`, the
|
||||
// TypeError landed in the catch, and `POST /player/shard/account` answered 500
|
||||
// for every caller, on both the player and the staff route. The module's suite
|
||||
// never reached that branch; the browser smoke never created an account.
|
||||
//
|
||||
// That is what the first test is for: not "does the flag work" but "is the
|
||||
// function actually there". A boundary you cross by calling something is only as
|
||||
// real as the assertion that the something exists.
|
||||
|
||||
const { test, afterEach } = require('node:test')
|
||||
const assert = require('node:assert/strict')
|
||||
|
||||
const { ctx } = require('./_setup')
|
||||
const gameSignup = require('../utils/gameSignup')
|
||||
const playerShard = require('../router/player/shard.controller')
|
||||
const publicShard = require('../router/public/shard.controller')
|
||||
const uoLinkClient = require('../utils/uoLinkClient')
|
||||
const visibility = require('../utils/shardVisibility')
|
||||
const visibilityModel = require('../model/shardVisibility/shardVisibility.model')
|
||||
|
||||
const originalGet = ctx.settings.get
|
||||
const originalSet = ctx.settings.set
|
||||
const originalCreate = uoLinkClient.createAccount
|
||||
const originalListAll = visibilityModel.listAll
|
||||
const originalViewerLevel = visibility.viewerLevel
|
||||
|
||||
afterEach(() => {
|
||||
ctx.settings.get = originalGet
|
||||
ctx.settings.set = originalSet
|
||||
uoLinkClient.createAccount = originalCreate
|
||||
visibilityModel.listAll = originalListAll
|
||||
visibility.viewerLevel = originalViewerLevel
|
||||
})
|
||||
|
||||
function mockRes() {
|
||||
return {
|
||||
statusCode: 200,
|
||||
body: null,
|
||||
status(c) { this.statusCode = c; return this },
|
||||
json(b) { this.body = b; return this },
|
||||
}
|
||||
}
|
||||
|
||||
const asPlayer = (body) => ({ body, user: { id: 7, username: 'kelmo', role: 'player' }, ip: '203.0.113.9' })
|
||||
|
||||
// ── The regression ─────────────────────────────────────────────────────────
|
||||
|
||||
test('creating a game account does not 500 when the site permits it', async () => {
|
||||
// The shape of the slice-1 bug: this route answered 500 for everyone because
|
||||
// the gate it called did not exist. Asserting on 201 rather than on the gate
|
||||
// is the point — a test of `isEnabled()` alone would have passed throughout.
|
||||
ctx.settings.get = async () => 'hybrid'
|
||||
uoLinkClient.createAccount = async () => ({ ok: true })
|
||||
|
||||
const res = mockRes()
|
||||
await playerShard.createGameAccount(asPlayer({ account: 'kelmo', password: 'hunter2hunter2' }), res)
|
||||
|
||||
assert.equal(res.statusCode, 201)
|
||||
assert.deepEqual(res.body, { account: 'kelmo', linked: true })
|
||||
})
|
||||
|
||||
test('the gate the controller calls is a function that exists', () => {
|
||||
// The assertion the module was missing. `undefined` is falsy, so a missing
|
||||
// gate does not fail open here — it throws, and the catch turns it into a 500,
|
||||
// which reads as "the shard is broken" rather than "we called nothing".
|
||||
assert.equal(typeof gameSignup.isEnabled, 'function')
|
||||
})
|
||||
|
||||
// ── The policy ─────────────────────────────────────────────────────────────
|
||||
|
||||
test('only website and hybrid offer signup; everything else is disabled', async () => {
|
||||
const answers = {}
|
||||
for (const mode of [...gameSignup.MODES, 'nonsense', null]) {
|
||||
ctx.settings.get = async () => mode
|
||||
answers[String(mode)] = await gameSignup.isEnabled()
|
||||
}
|
||||
assert.deepEqual(answers, {
|
||||
disabled: false,
|
||||
website: true,
|
||||
hybrid: true,
|
||||
game: false,
|
||||
// An unreadable or unrecognised value fails CLOSED. Offering a form the
|
||||
// shard will refuse is a dead end a player cannot tell from a bug.
|
||||
nonsense: false,
|
||||
null: false,
|
||||
})
|
||||
})
|
||||
|
||||
test('signup is refused with 403, not 500, when the site does not offer it', async () => {
|
||||
ctx.settings.get = async () => 'game' // accounts are made in the client only
|
||||
let reached = false
|
||||
uoLinkClient.createAccount = async () => { reached = true; return { ok: true } }
|
||||
|
||||
const res = mockRes()
|
||||
await playerShard.createGameAccount(asPlayer({ account: 'kelmo', password: 'hunter2hunter2' }), res)
|
||||
|
||||
assert.equal(res.statusCode, 403)
|
||||
assert.equal(reached, false, 'the shard must not be called when the site refuses')
|
||||
})
|
||||
|
||||
test('setMode refuses a mode that is not one of the four', async () => {
|
||||
let written = null
|
||||
ctx.settings.set = async (key, value) => { written = { key, value } }
|
||||
|
||||
await gameSignup.setMode('hybrid', 3)
|
||||
assert.deepEqual(written, { key: 'game_account_signup', value: 'hybrid' })
|
||||
|
||||
await assert.rejects(() => gameSignup.setMode('everyone', 3), /unknown game-signup mode/)
|
||||
assert.deepEqual(written, { key: 'game_account_signup', value: 'hybrid' }, 'nothing was written')
|
||||
})
|
||||
|
||||
test('the setting key is unchanged, so an existing instance keeps its mode', () => {
|
||||
// Not a style assertion. Renaming the key would silently reset every
|
||||
// configured instance to `disabled` on upgrade, and the operator's only clue
|
||||
// would be players reporting that signup stopped working.
|
||||
assert.equal(gameSignup.KEY, 'game_account_signup')
|
||||
})
|
||||
|
||||
// ── The client's view of it ────────────────────────────────────────────────
|
||||
|
||||
test('public features carries gameAccountSignup, and it is not audience-gated', async () => {
|
||||
// The portal and the invite step both read this. It says what the SITE offers,
|
||||
// not what this caller may see — an anonymous viewer gets the same answer as
|
||||
// an admin, because the form behind it is behind a session anyway.
|
||||
visibilityModel.listAll = async () => []
|
||||
ctx.settings.get = async () => 'website'
|
||||
|
||||
const answers = []
|
||||
for (const level of ['anonymous', 'admin']) {
|
||||
visibility.viewerLevel = async () => level
|
||||
const res = mockRes()
|
||||
await publicShard.getFeatures({}, res)
|
||||
answers.push(res.body.gameAccountSignup)
|
||||
}
|
||||
|
||||
assert.deepEqual(answers, [true, true])
|
||||
})
|
||||
|
||||
test('a features read still answers when the signup setting cannot be read', async () => {
|
||||
// Nav gating is the endpoint's main job and it must not be taken down by the
|
||||
// one boolean bolted onto it. `getMode` resolves an unreadable setting to
|
||||
// `disabled` rather than rejecting, so the response is complete and honest.
|
||||
visibilityModel.listAll = async () => []
|
||||
visibility.viewerLevel = async () => 'anonymous'
|
||||
ctx.settings.get = async () => { throw new Error('settings table is on fire') }
|
||||
|
||||
const res = mockRes()
|
||||
await publicShard.getFeatures({}, res)
|
||||
|
||||
assert.equal(res.statusCode, 500, 'a throwing settings read is a real failure, reported as one')
|
||||
})
|
||||
Reference in New Issue
Block a user