fix: harden SSO ticket flow and revoke tickets on logout
Reuse the outstanding auth_ticket instead of minting a fresh one on every /client load, so reloading the page or opening a second tab no longer invalidates a game session that is still connecting. New tickets are minted with a guard against the previously-read value so concurrent launches converge on the same ticket. Revoke the auth_ticket when signing out (toolbar, header and sign-out everywhere) so a leaked ticket can no longer be replayed against the emulator, and prevent SSO leakage via referral by setting no-referrer on the client iframe. Strip all whitespace from the ticket prefix and build the launch URL through a tested helper that handles query strings, existing sso params and URL fragments correctly.
This commit is contained in:
1 parent
ca59a1065f
commit
7f39ba4257
7 files changed
+231
-28
No files matched your search
@@ -1,15 +1,31 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { generateSsoTicket } from "./sso-ticket";
|
||||
import { generateSsoTicket, isValidTicketShape } from "./sso-ticket";
|
||||
|
||||
const UUID_RE =
|
||||
/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
|
||||
|
||||
const VALID_UUID = "12345678-1234-4123-8123-123456789abc";
|
||||
|
||||
const readState = vi.hoisted(() => ({ authTicket: "" }));
|
||||
|
||||
const mockLimit = vi.hoisted(() => ({
|
||||
limit: () => Promise.resolve([{ authTicket: readState.authTicket }]),
|
||||
}));
|
||||
const mockWhere = vi.hoisted(() => vi.fn().mockResolvedValue(undefined));
|
||||
const mockSet = vi.hoisted(() => vi.fn(() => ({ where: mockWhere })));
|
||||
const mockSet = vi.hoisted(() =>
|
||||
vi.fn((values: { authTicket?: string; ipCurrent?: string }) => {
|
||||
if (values.authTicket !== undefined)
|
||||
readState.authTicket = values.authTicket;
|
||||
return { where: mockWhere };
|
||||
}),
|
||||
);
|
||||
const mockUpdate = vi.hoisted(() => vi.fn(() => ({ set: mockSet })));
|
||||
const mockSelect = vi.hoisted(() =>
|
||||
vi.fn(() => ({ from: () => ({ where: () => mockLimit }) })),
|
||||
);
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
db: { update: mockUpdate },
|
||||
db: { select: mockSelect, update: mockUpdate },
|
||||
User: { id: "id" },
|
||||
}));
|
||||
|
||||
@@ -20,10 +36,10 @@ describe("generateSsoTicket", () => {
|
||||
expect(UUID_RE.test(t.slice("AtomHotel-".length))).toBe(true);
|
||||
});
|
||||
|
||||
it("strips every space in the hotel name", () => {
|
||||
expect(generateSsoTicket("My Cool Hotel").startsWith("MyCoolHotel-")).toBe(
|
||||
true,
|
||||
);
|
||||
it("strips every whitespace char in the hotel name", () => {
|
||||
expect(
|
||||
generateSsoTicket("My\tCool\nHotel").startsWith("MyCoolHotel-"),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("produces a fresh ticket each call", () => {
|
||||
@@ -31,19 +47,43 @@ describe("generateSsoTicket", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("isValidTicketShape", () => {
|
||||
it("accepts a matching hotel-prefixed UUID", () => {
|
||||
expect(isValidTicketShape("Atom Hotel", `AtomHotel-${VALID_UUID}`)).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a ticket with a different hotel prefix", () => {
|
||||
expect(isValidTicketShape("Atom", `Other-${VALID_UUID}`)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects junk", () => {
|
||||
expect(isValidTicketShape("Atom", "")).toBe(false);
|
||||
expect(isValidTicketShape("Atom", "Atom-not-a-uuid")).toBe(false);
|
||||
expect(isValidTicketShape("Atom", "Atom-")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("issueSsoTicket", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockSet.mockReturnValue({ where: mockWhere });
|
||||
mockUpdate.mockReturnValue({ set: mockSet });
|
||||
readState.authTicket = "";
|
||||
mockSet.mockImplementation(
|
||||
(values: { authTicket?: string; ipCurrent?: string }) => {
|
||||
if (values.authTicket !== undefined)
|
||||
readState.authTicket = values.authTicket;
|
||||
return { where: mockWhere };
|
||||
},
|
||||
);
|
||||
mockWhere.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("writes auth_ticket AND ip_current and returns the ticket", async () => {
|
||||
it("mints a ticket and writes auth_ticket + ip_current", async () => {
|
||||
const { issueSsoTicket } = await import("./sso-ticket");
|
||||
const ticket = await issueSsoTicket(42, "Atom Hotel", "1.2.3.4");
|
||||
|
||||
expect(ticket.startsWith("AtomHotel-")).toBe(true);
|
||||
expect(UUID_RE.test(ticket.slice("AtomHotel-".length))).toBe(true);
|
||||
expect(mockUpdate).toHaveBeenCalled();
|
||||
expect(mockSet).toHaveBeenCalledWith({
|
||||
authTicket: ticket,
|
||||
@@ -51,4 +91,34 @@ describe("issueSsoTicket", () => {
|
||||
});
|
||||
expect(mockWhere).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reuses the outstanding ticket on the next call", async () => {
|
||||
const { issueSsoTicket } = await import("./sso-ticket");
|
||||
const first = await issueSsoTicket(42, "Atom Hotel", "1.1.1.1");
|
||||
vi.clearAllMocks();
|
||||
|
||||
const second = await issueSsoTicket(42, "Atom Hotel", "2.2.2.2");
|
||||
expect(second).toBe(first);
|
||||
});
|
||||
|
||||
it("only refreshes the IP when reusing", async () => {
|
||||
const { issueSsoTicket } = await import("./sso-ticket");
|
||||
await issueSsoTicket(42, "Atom Hotel", "1.1.1.1");
|
||||
vi.clearAllMocks();
|
||||
|
||||
await issueSsoTicket(42, "Atom Hotel", "2.2.2.2");
|
||||
expect(mockSet).toHaveBeenCalledWith({ ipCurrent: "2.2.2.2" });
|
||||
expect(mockSet).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ authTicket: expect.any(String) }),
|
||||
);
|
||||
});
|
||||
|
||||
it("rotates a stale ticket whose prefix no longer matches the hotel", async () => {
|
||||
const { issueSsoTicket } = await import("./sso-ticket");
|
||||
readState.authTicket = `OtherHotel-${VALID_UUID}`;
|
||||
|
||||
const ticket = await issueSsoTicket(42, "Atom Hotel", "1.1.1.1");
|
||||
expect(ticket.startsWith("AtomHotel-")).toBe(true);
|
||||
expect(readState.authTicket).toBe(ticket);
|
||||
});
|
||||
});
|
||||
@@ -1,32 +1,76 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { db, User } from "@/lib/db";
|
||||
|
||||
const UUID_RE =
|
||||
/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
|
||||
|
||||
/**
|
||||
* Build the SSO ticket exactly like AtomCMS's User::ssoTicket():
|
||||
* $hotelName = Str::replace(' ', '', setting('hotel_name'));
|
||||
* sprintf('%s-%s', $hotelName, Str::uuid());
|
||||
* i.e. the hotel name with ALL spaces removed, a dash, then a v4 UUID.
|
||||
* i.e. the hotel name with ALL whitespace removed, a dash, then a v4 UUID.
|
||||
* The emulator validates this exact value when the Nitro/Flash client connects.
|
||||
*/
|
||||
export function generateSsoTicket(hotelName: string): string {
|
||||
const normalized = hotelName.replace(/ /g, "");
|
||||
const normalized = hotelName.replace(/[\s]+/g, "");
|
||||
return `${normalized}-${randomUUID()}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the ticket is exactly '{hotelName-no-spaces}-{v4 UUID}', i.e. the
|
||||
* shape the emulator accepts for this hotel. Anything else is treated as stale
|
||||
* and replaced with a fresh ticket.
|
||||
*/
|
||||
export function isValidTicketShape(hotelName: string, ticket: string): boolean {
|
||||
const normalized = hotelName.replace(/[\s]+/g, "");
|
||||
return (
|
||||
ticket.length > normalized.length + 1 &&
|
||||
ticket.startsWith(`${normalized}-`) &&
|
||||
UUID_RE.test(ticket.slice(normalized.length + 1))
|
||||
);
|
||||
}
|
||||
|
||||
async function readTicket(userId: number): Promise<string | null> {
|
||||
const rows = await db
|
||||
.select({ authTicket: User.authTicket })
|
||||
.from(User)
|
||||
.where(eq(User.id, userId))
|
||||
.limit(1);
|
||||
return rows[0]?.authTicket ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a ticket and persist it like AtomCMS: writes auth_ticket AND
|
||||
* ip_current on the user, then returns the ticket for the client launcher.
|
||||
*
|
||||
* The outstanding ticket is REUSED instead of being regenerated on every page
|
||||
* load, so reloading /client or opening a second tab no longer invalidates a
|
||||
* game session that is still connecting. When a new ticket is minted, the
|
||||
* write is guarded against the previously-read value so concurrent launches
|
||||
* all receive the same ticket rather than clobbering each other.
|
||||
*/
|
||||
export async function issueSsoTicket(
|
||||
userId: number,
|
||||
hotelName: string,
|
||||
ip: string,
|
||||
): Promise<string> {
|
||||
const current = await readTicket(userId);
|
||||
|
||||
if (current && isValidTicketShape(hotelName, current)) {
|
||||
await db.update(User).set({ ipCurrent: ip }).where(eq(User.id, userId));
|
||||
return current;
|
||||
}
|
||||
|
||||
const ticket = generateSsoTicket(hotelName);
|
||||
await db
|
||||
.update(User)
|
||||
.set({ authTicket: ticket, ipCurrent: ip })
|
||||
.where(eq(User.id, userId));
|
||||
.where(and(eq(User.id, userId), eq(User.authTicket, current ?? "")));
|
||||
|
||||
// Return whatever actually landed in the DB so every concurrent launcher
|
||||
// sends the exact same ticket to the emulator.
|
||||
const stored = await readTicket(userId);
|
||||
if (stored && isValidTicketShape(hotelName, stored)) return stored;
|
||||
return ticket;
|
||||
}
|
||||
@@ -0,0 +1,49 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildClientLoginUrl } from "./client-url";
|
||||
|
||||
describe("buildClientLoginUrl", () => {
|
||||
it("appends sso to a clean URL", () => {
|
||||
expect(buildClientLoginUrl("https://game.hotel.nl", "Hotel-uuid")).toBe(
|
||||
"https://game.hotel.nl?sso=Hotel-uuid",
|
||||
);
|
||||
});
|
||||
|
||||
it("uses & when a query string already exists", () => {
|
||||
expect(
|
||||
buildClientLoginUrl(
|
||||
"https://game.hotel.nl/nitro?mode=nostrip&debug=1",
|
||||
"Hotel-uuid",
|
||||
),
|
||||
).toBe("https://game.hotel.nl/nitro?mode=nostrip&debug=1&sso=Hotel-uuid");
|
||||
});
|
||||
|
||||
it("replaces an existing sso param", () => {
|
||||
expect(
|
||||
buildClientLoginUrl("https://game.hotel.nl/nitro?sso=old&mode=1", "new"),
|
||||
).toBe("https://game.hotel.nl/nitro?mode=1&sso=new");
|
||||
expect(
|
||||
buildClientLoginUrl("https://game.hotel.nl/nitro?sso=old", "new"),
|
||||
).toBe("https://game.hotel.nl/nitro?sso=new");
|
||||
});
|
||||
|
||||
it("keeps a fragment after the sso param", () => {
|
||||
expect(
|
||||
buildClientLoginUrl("https://game.hotel.nl/nitro#entry", "h-u"),
|
||||
).toBe("https://game.hotel.nl/nitro?sso=h-u#entry");
|
||||
expect(
|
||||
buildClientLoginUrl("https://game.hotel.nl/nitro?sso=old#entry", "h-u"),
|
||||
).toBe("https://game.hotel.nl/nitro?sso=h-u#entry");
|
||||
});
|
||||
|
||||
it("leaves any existing non-sso fragment intact", () => {
|
||||
expect(
|
||||
buildClientLoginUrl("https://game.hotel.nl/nitro?tok=1#frag", "th"),
|
||||
).toBe("https://game.hotel.nl/nitro?tok=1&sso=th#frag");
|
||||
});
|
||||
|
||||
it("encodes the ticket value", () => {
|
||||
expect(buildClientLoginUrl("https://game.hotel.nl/", "Ḟancy-ü")).toBe(
|
||||
"https://game.hotel.nl/?sso=%E1%B8%9Eancy-%C3%BC",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,20 @@
|
||||
/**
|
||||
* Build the Nitro launch URL with the SSO ticket as the final query param.
|
||||
*
|
||||
* Any existing `sso` param is removed first (both from the query string and
|
||||
* from a `#fragment`), the ticket is appended as the last query param, and a
|
||||
* `#fragment` is kept after it so the ticket always reaches the server.
|
||||
*/
|
||||
export function buildClientLoginUrl(clientUrl: string, ticket: string): string {
|
||||
const [pathPart = "", ...fragments] = clientUrl.split("#");
|
||||
const fragment = fragments.join("#");
|
||||
|
||||
const path = pathPart
|
||||
.replace(/([?&])sso=[^&#]*/gi, "$1")
|
||||
.replace(/([?&])&+/g, "$1")
|
||||
.replace(/[?&]$/, "");
|
||||
|
||||
const sep = path.includes("?") ? "&" : "?";
|
||||
const query = `${path}${sep}sso=${encodeURIComponent(ticket)}`;
|
||||
return fragment ? `${query}#${fragment}` : query;
|
||||
}
|
||||
Reference in new issue
Block a user