diff --git a/server/router/admin/uoLink.controller.js b/server/router/admin/uoLink.controller.js index 4075448..9122a79 100644 --- a/server/router/admin/uoLink.controller.js +++ b/server/router/admin/uoLink.controller.js @@ -10,6 +10,7 @@ const uoLinkConfig = require('../../model/uoLinkConfig/uoLinkConfig.model') const uoLinkClient = require('../../utils/uoLinkClient') const uoLinkSocket = require('../../utils/uoLinkSocket') const shardBroadcast = require('../../utils/shardBroadcast') +const gameSignup = require('../../utils/gameSignup') const { activity } = require('../../core') const log = require('../../core').logger('admin-uolink') @@ -75,6 +76,34 @@ async function saveConfig(req, res) { } } +// GET /admin/uo-link/signup-mode — whether this site creates game accounts. +// +// Core's Site Settings carried this field until slice 3, with help text naming +// Bridge.cfg. It reads as UO policy because it is: the site's mode and the +// shard's own SignupMode have to agree, and only one of those two is core's. +async function getSignupMode(req, res) { + try { + return res.json({ mode: await gameSignup.getMode(), modes: gameSignup.MODES }) + } catch (err) { + log.error('uoLink.getSignupMode', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// PUT /admin/uo-link/signup-mode +async function saveSignupMode(req, res) { + const { mode } = req.body + try { + await gameSignup.setMode(mode, req.user.id) + await activity.log({ req, action: 'uoLink.signupMode.update', detail: { mode } }) + log.info('game-signup mode updated', { by: req.user.username, mode }) + return res.json({ mode }) + } catch (err) { + log.error('uoLink.saveSignupMode', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + // POST /admin/uo-link/towncrier — publish/replace a town-crier message. async function postTownCrier(req, res) { const { id, lines, durationSec } = req.body @@ -120,4 +149,4 @@ function stream(req, res) { shardBroadcast.subscribe(req, res, 'admin') } -module.exports = { getConfig, saveConfig, postTownCrier, deleteTownCrier, stream } +module.exports = { getConfig, saveConfig, getSignupMode, saveSignupMode, postTownCrier, deleteTownCrier, stream } diff --git a/server/router/admin/uoLink.router.js b/server/router/admin/uoLink.router.js index 8974733..80fa632 100644 --- a/server/router/admin/uoLink.router.js +++ b/server/router/admin/uoLink.router.js @@ -24,6 +24,7 @@ const express = core.express const { body, param } = core.validator const uoLink = require('./uoLink.controller') +const gameSignup = require('../../utils/gameSignup') const { requireRole, validate } = core.middleware const uoLinkRouter = express.Router() @@ -58,6 +59,42 @@ uoLinkRouter.put( validate, uoLink.saveConfig, ) + +// ── Game-account signup mode ─────────────────────────────────────────────── +// +// New in slice 3, and new only in the sense that the field moved: core's Site +// Settings has carried `game_account_signup` since long before the extraction, +// and its help text has always been about a game server. The setting key and its +// stored value are unchanged, so an existing instance keeps its configured mode. +uoLinkRouter.get( + '/signup-mode', + // #swagger.tags = ['Admin · Shard'] + // #swagger.summary = 'Get the game-account signup mode (admin only)' + // #swagger.description = 'Whether the site offers game-account creation, and in which direction. The shard\'s own SignupMode (Bridge.cfg) must agree: website/hybrid accept site-created accounts, game refuses them.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'The configured mode and the legal values', content: { "application/json": { schema: { type: "object", properties: { mode: { type: "string" }, modes: { type: "array", items: { type: "string" } } } } } } } */ + /* #swagger.responses[403] = { description: 'Admin role required', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + uoLink.getSignupMode, +) +uoLinkRouter.put( + '/signup-mode', + // #swagger.tags = ['Admin · Shard'] + // #swagger.summary = 'Set the game-account signup mode (admin only)' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: "object", required: ["mode"], properties: { mode: { type: "string", enum: ["disabled","website","hybrid","game"] } } } } } } */ + /* #swagger.responses[200] = { description: 'The saved mode', content: { "application/json": { schema: { type: "object", properties: { mode: { type: "string" } } } } } } */ + /* #swagger.responses[400] = { description: 'Unknown mode', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[403] = { description: 'Admin role required', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + // Validated here as well as in gameSignup.setMode: the list is the same list, + // and the difference is the answer. A rejected value must be a 400 naming the + // field, not a 500 from a thrown Error the controller could only guess about. + body('mode').isIn(gameSignup.MODES), + validate, + uoLink.saveSignupMode, +) + uoLinkRouter.post( '/towncrier', // #swagger.tags = ['Admin · Shard'] diff --git a/server/router/public/shard.controller.js b/server/router/public/shard.controller.js index 7b70ad2..cfa1357 100644 --- a/server/router/public/shard.controller.js +++ b/server/router/public/shard.controller.js @@ -15,6 +15,7 @@ const shardMarket = require('../../model/shardMarket/shardMarket.model') const uoLinkConfig = require('../../model/uoLinkConfig/uoLinkConfig.model') const broadcast = require('../../utils/shardBroadcast') const visibility = require('../../utils/shardVisibility') +const gameSignup = require('../../utils/gameSignup') const log = require('../../core').logger('public-shard') @@ -386,7 +387,20 @@ async function getFeatures(req, res) { try { const config = await visibility.getConfig() const level = await visibility.viewerLevel(req) - return res.json({ level, features: visibility.visibleFeatures(level, config) }) + return res.json({ + level, + features: visibility.visibleFeatures(level, config), + // Whether this site offers game-account creation. Not a visibility flag + // and deliberately carried here anyway: it is the same per-viewer, + // once-a-session answer, and the alternative is a second endpoint and a + // second round-trip for one boolean. It is NOT audience-gated — it says + // what the site offers, not what this caller may see, and the portal's + // create-account form is behind a session either way. + // + // Core's public settings carried this until slice 3. It is ours now + // (utils/gameSignup.js), because the setting is about a game server. + gameAccountSignup: await gameSignup.isEnabled(), + }) } catch (err) { log.error('shard.getFeatures', err) return res.status(500).json({ message: 'Internal Server Error' }) diff --git a/server/test/gameSignup.test.js b/server/test/gameSignup.test.js new file mode 100644 index 0000000..1da5b85 --- /dev/null +++ b/server/test/gameSignup.test.js @@ -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') +}) diff --git a/server/utils/gameSignup.js b/server/utils/gameSignup.js new file mode 100644 index 0000000..03be42e --- /dev/null +++ b/server/utils/gameSignup.js @@ -0,0 +1,63 @@ +// ── Whether this site creates game accounts, and in which direction ──────── +// +// This policy was core's until slice 3 of the Phase 3 extraction, and it should +// never have been: the setting's own help text names *Bridge.cfg* and says the +// game server's `SignupMode` must agree with it. That is a sentence about a UO +// shard, and core cannot own a sentence about a UO shard. +// +// **The setting key is unchanged.** `game_account_signup` keeps its name and its +// row in core's `settings` table, read and written through `ctx.settings`. The +// key is not prefixed because renaming it would silently reset every existing +// instance's configured mode to the default — the same reasoning that +// grandfathered `spawn_atlas_servuo_path`, `cliloc_client_path` and the seven +// stream ids (MODULE_API.md §6.5). A module owning an unprefixed settings key is +// a grandfathering, not a pattern to copy. +// +// **It was also broken.** Slice 1 ported the call site +// (`router/player/shard.controller.js`) still calling +// `settings.isGameAccountSignupEnabled()`, which `ctx.settings` does not expose — +// it is three functions, not the model. So `POST /player/shard/account` threw a +// TypeError and answered 500 for every caller, and no test saw it because the +// module's suite never reached that branch. This file is where that function now +// lives, on the side that actually uses it. + +const { settings } = require('../core') + +const KEY = 'game_account_signup' + +/** + * The four modes, and what each means. + * + * `website` and `hybrid` are the two that accept a site-created account; `game` + * means accounts are made in the client and only linked here. The shard's own + * `SignupMode` still has the final say when the call is actually made — this is + * the site half of an agreement between two systems, which is exactly why it + * reads as UO policy rather than as site configuration. + */ +const MODES = ['disabled', 'website', 'hybrid', 'game'] +const OFFERS_SIGNUP = ['website', 'hybrid'] + +/** The configured mode, or `disabled` for anything unset or unrecognised. */ +async function getMode() { + const value = await settings.get(KEY) + return MODES.includes(value) ? value : 'disabled' +} + +/** + * Does this site offer game-account creation right now? + * + * Fails CLOSED on an unreadable setting, because `getMode` resolves an unknown + * value to `disabled`. Offering a form that the shard will refuse is a dead end + * a player cannot distinguish from a bug. + */ +async function isEnabled() { + return OFFERS_SIGNUP.includes(await getMode()) +} + +/** @throws if `mode` is not one of MODES — the caller validates first. */ +async function setMode(mode, updatedBy) { + if (!MODES.includes(mode)) throw new Error(`unknown game-signup mode "${mode}"`) + return settings.set(KEY, mode, updatedBy) +} + +module.exports = { KEY, MODES, OFFERS_SIGNUP, getMode, isEnabled, setMode }