Skip to content

Commit 79c92f6

Browse files
committed
fix a bug renaming bags and adjust referer checks
1 parent c9f4597 commit 79c92f6

12 files changed

Lines changed: 108 additions & 40 deletions

File tree

ARCHITECTURE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,4 @@ These are the hooks that run on startup. The listen command starts the server an
126126

127127
- improve the small screen form factor for the admin UI
128128
- make sure all the descriptions and labels make sense
129+
- something seems to be bugged in upserts

packages/mws/src/RequestState.ts

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { ServerState } from "./ServerState";
44
import { BodyFormat, ParsedRequest, RouteMatch, Router, ServerRequest, Streamer, StreamerHeadersInput, SuperHeadersInit, SuperHeadersPropertyInit, truthy } from "@tiddlywiki/server";
55
import { SendError, SendErrorReasonData } from "@tiddlywiki/server";
66
import { ServerToReactAdmin } from './services/setupDevServer';
7-
import { AuthUser } from './services/sessions';
7+
import { AuthUser } from './new-managers/sessions';
88

99

1010
declare module "@tiddlywiki/server" {
@@ -60,13 +60,13 @@ export class StateObject<
6060

6161
okUser() {
6262
if (!this.user.isLoggedIn)
63-
throw new SendError("ACCESS_DENIED", 403, { reason: "User not authenticated" })
63+
throw new SendError("ACCESS_DENIED", 403, { reason: "User not authenticated" })
6464
}
6565
okAdmin() {
66-
if (!this.user.isLoggedIn)
67-
throw new SendError("ACCESS_DENIED", 403, { reason: "User not authenticated" })
68-
if (!this.user.isAdmin)
69-
throw new SendError("ACCESS_DENIED", 403, { reason: "User is not an admin" })
66+
if (!this.user.isLoggedIn)
67+
throw new SendError("ACCESS_DENIED", 403, { reason: "User not authenticated" })
68+
if (!this.user.isAdmin)
69+
throw new SendError("ACCESS_DENIED", 403, { reason: "User is not an admin" })
7070
}
7171

7272
async $transaction<T>(fn: (prisma: PrismaTxnClient) => Promise<T>): Promise<T> {
@@ -81,32 +81,47 @@ export class StateObject<
8181
return this.engine.$transaction(arg(this.engine), options);
8282
}
8383

84+
/**
85+
* This is intended to prevent malicious code attempting to access restricted
86+
* bags using the credentials of more privelaged users and then saving the data
87+
* to less restricted bags. Eventually this is intended to have a more complete
88+
* system where a page requests permission to access another page on behalf of the user.
89+
*/
8490
assertWikiReferer(recipe_slug: string) {
8591
const state = this;
8692
if (!state.headers.referer) return;
8793
const referer = new URL(state.headers.referer);
88-
// console.log("Referer", state.headers.referer, referer);
94+
// for now pages outside the path prefix are denied, but this should probably change
8995
if (!referer.pathname.startsWith(state.pathPrefix))
90-
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
96+
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
97+
// if a recipe endpoint somehow serves a page, it's definitely wiki content, so deny it
98+
if (referer.pathname.startsWith(state.pathPrefix + "/recipe/"))
99+
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
100+
// pages outside the wiki path are allowed to access wiki endpoints if they want
91101
if (!referer.pathname.startsWith(state.pathPrefix + "/wiki/"))
92-
return; // keep going
102+
return;
93103
// we now get the recipe name from the referer
94104
const recipe_name = referer.pathname.substring(state.pathPrefix.length + "/wiki/".length);
95105
const referer_slug = decodeURIComponent(recipe_name) as PrismaField<"Recipe", "id">;
96106
if (referer_slug !== recipe_slug)
97-
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
107+
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
98108
}
99109

100-
assertAdminReferer() {
110+
/**
111+
* If this throws in the wrong situation it may be a bug.
112+
* Normally this is intended to keep wiki code from trying to acces admin or login paths,
113+
* since malicious code would be accessing it from every computer that opens the wiki.
114+
* It is not intended to restrict legitimate scenarios, especially not scenarios coded into
115+
* the admin-vanilla client. If it throws under stock conditions, it's a bug.
116+
*/
117+
assertRefererPrefix(prefix: string[]) {
101118
const state = this;
102119
// referer is a voluntary header
103120
if (!state.headers.referer) return;
104121
const referer = new URL(state.headers.referer);
105122
// deny referers from outside our pathprefix
106-
if (!referer.pathname.startsWith(state.pathPrefix))
107-
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
108-
if (referer.pathname.startsWith(state.pathPrefix + "/wiki/"))
109-
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
123+
if (!prefix.some(e => referer.pathname.startsWith(state.pathPrefix + e)))
124+
throw new SendError("ACCESS_DENIED", 403, { reason: "Referer check failed" })
110125
}
111126

112127

packages/mws/src/ServerState.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import { ITXClientDenyList } from "@tiddlywiki/mws-prisma";
33
import { TW } from "tiddlywiki";
44
import pkg from "../package.json";
55
import { createPasswordService } from "./services/PasswordService";
6-
import { startupCache } from "./services/cache";
6+
import { startupCache } from "./new-managers/plugin-cache";
77
import { Types } from "@tiddlywiki/mws-prisma";
88
import { dist_resolve } from "@tiddlywiki/server";
99
import { readFileSync } from "fs";
10-
import { SessionManager, SessionManagerObject } from "./services/sessions";
10+
import { SessionManager, SessionManagerObject } from "./new-managers/sessions";
1111

1212
/** This is an alias for ServerState in case we want to separate the two purposes. */
1313
export type SiteConfig = ServerState;

packages/mws/src/index.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import "./zodAssert";
1616
import "./RequestState";
1717
import "./ServerState";
1818
import "./services/tw-routes";
19-
import "./services/cache";
20-
import "./services/sessions";
19+
import "./new-managers/plugin-cache";
20+
import "./new-managers/sessions";
2121
import "./SendError";
2222

2323
// startup
@@ -33,7 +33,7 @@ export {
3333
SessionManager,
3434
SessionManagerObject,
3535
AuthUser
36-
} from "./services/sessions";
36+
} from "./new-managers/sessions";
3737
export {
3838
PasswordService
3939
} from "./services/PasswordService";
@@ -42,7 +42,7 @@ export {
4242
PluginDefinition,
4343
TiddlerHasher,
4444
defaultPreloadFunction,
45-
} from "./services/cache";
45+
} from "./new-managers/plugin-cache";
4646
export * from "./services/setupDevServer";
4747
export * from "./services/tiddlywiki";
4848
export {

packages/mws/src/new-commands/load-wiki-folder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { BaseCommand, CommandInfo } from "@tiddlywiki/commander";
2-
import { WikiPluginCache } from "../services/cache";
2+
import { WikiPluginCache } from "../new-managers/plugin-cache";
33
import { TiddlerFields, TW } from "tiddlywiki";
44
import * as fs from "fs";
55
import * as path from "path";

packages/mws/src/new-managers/RecipeResolver.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { join, resolve } from "node:path";
1515
import { mapGetInit } from "./wiki-utils";
1616
import { IdString } from "@mws/admin-vanilla/src/definition/tabs";
1717
import { serverEvents } from "@tiddlywiki/events";
18-
import { defaultPreloadFunction, TiddlerHasher, WikiPluginCache } from "../services/cache";
18+
import { defaultPreloadFunction, TiddlerHasher, WikiPluginCache } from "./plugin-cache";
1919

2020
// ---------------------------------------------------------------------------
2121
// Types

packages/mws/src/new-managers/TabDataAdapter.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ import {
3333
UpsertTemplateInput
3434
} from "./wiki-contract";
3535
import { createHash } from "crypto";
36+
import { debuglog } from "util";
37+
import { Debug } from "@prisma/client/runtime/client";
38+
39+
3640

3741
type IRecipeRow = DataStore["wikis"][number];
3842
type ITemplateRow = DataStore["templates"][number];
@@ -532,7 +536,7 @@ export const AdminSave = zodRoute({
532536
}),
533537
zodRequestBody: z => z.any(),
534538
inner: async (state) => {
535-
state.assertAdminReferer();
539+
state.assertRefererPrefix(["/admin/"]);
536540
state.asserted = state.user.isLoggedIn;
537541
state.data = JSON.parse(JSON.stringify(state.data), (key: any, val: any) => {
538542
if (typeof val === "string" && val.startsWith(IdString.prefix))
@@ -584,7 +588,7 @@ export const AdminLoad = zodRoute({
584588
securityChecks: { requestedWithHeader: true },
585589
zodPathParams: z => ({}),
586590
inner: async (state) => {
587-
state.assertAdminReferer();
591+
state.assertRefererPrefix(["/admin/"]);
588592
state.asserted = state.user.isLoggedIn;
589593
const res = await state.$transaction(async prisma => {
590594
const roles = await new RoleImportWriter(prisma, false).getIdMapper();

packages/mws/src/new-managers/TabUpserts.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
import { Prisma } from "@tiddlywiki/mws-prisma";
1212
import { mapGetInit, thrower } from "./wiki-utils";
1313
import { IdString, TabId } from "@mws/admin-vanilla/src/definition/tabs";
14+
import { Debug } from "@prisma/client/runtime/client";
1415

1516

1617
export type ImportedRoleRows = Awaited<ReturnType<PrismaTxnClient["roles"]["findMany"]>>;
@@ -78,19 +79,22 @@ export abstract class PerClassImportWriter<Modal extends PrismaModalKeys> {
7879
protected initStore: boolean,
7980
) {
8081
if (initStore) this.asserted = true;
82+
this.debug = Debug("mws:admin:" + tabid)
8183
}
82-
84+
debug: ART<typeof Debug>;
8385
private asserted = false;
8486
assertPermissions() {
8587
if (!this.asserted)
8688
throw new Error("Permissions must be asserted first.")
8789
}
8890

8991
async checkExisting(id: IdString, name: string, user: ServerRequest["user"]) {
90-
if (user.isAdmin) { this.asserted = true; }
92+
if (user.isAdmin) { this.debug("isAdmin: true"); this.asserted = true; }
9193
if (this.initStore && typeof user.isLoggedIn === "boolean")
9294
throw new Error("This shouldn't happen.");
9395

96+
this.debug("id.toString()", !!id.toString());
97+
9498
if (!user.isAdmin && this.adminLevel) {
9599

96100
if (!id.toString())
@@ -127,21 +131,26 @@ export abstract class PerClassImportWriter<Modal extends PrismaModalKeys> {
127131
where: { [this.id]: id.toString() },
128132
select: { [this.id]: true, [this.name]: true }
129133
});
134+
this.debug("existing", existing[this.id], existing[this.name], name)
130135
if (!existing)
131136
throw new Error("existing wiki not found");
132-
if (existing[this.name] !== name)
137+
if (existing[this.name] !== name) {
133138
await this.rename([[existing[this.name], name]]);
139+
}
134140
}
135141

136142
}
137143

138144
async rename(renames: [string, string][]) {
139145
this.assertPermissions();
140-
await Promise.all(renames.map(([oldName, newName]) =>
141-
(this.tx[this.modal] as any).update({
146+
this.debug("renames", renames);
147+
await Promise.all(renames.map(async ([oldName, newName]) => {
148+
this.debug("rename", this.modal, this.name, oldName, newName);
149+
await (this.tx[this.modal] as any).update({
142150
where: { [this.name]: oldName },
143151
data: { [this.name]: newName }
144-
})));
152+
});
153+
}));
145154
}
146155
/** Maps names to ids */
147156
async getNameMapper(names?: readonly string[]) {
@@ -233,14 +242,15 @@ export class UserImportWriter extends PerClassImportWriter<"users"> {
233242

234243
export class BagImportWriter extends PerClassImportWriter<"bag"> {
235244
constructor(tx: PrismaTxnClient, initStore: boolean) {
236-
super(tx, "bags", "bag", "name", "id", "C_admin", initStore)
245+
super(tx, "bags", "bag", "name", "id", "C_admin", initStore);
237246
}
238247

239248
async rename(renames: [string, string][]) {
240249
this.assertPermissions();
241250
const renamesMap = new Map(renames);
251+
this.debug("renames", ...renamesMap.entries());
242252
const bags = await this.tx.bag.findMany({
243-
where: { name: { in: Array.from(renamesMap.keys(), e => e) } },
253+
where: { name: { in: Array.from(renamesMap.keys()) } },
244254
include: { recipe_bags: { select: { recipe: { select: { id: true, template_id: true } } } }, }
245255
});
246256

@@ -252,6 +262,8 @@ export class BagImportWriter extends PerClassImportWriter<"bag"> {
252262
mapGetInit(recipesInvolved, bag.name, () => new Set()).add(rb.recipe.id);
253263
mapGetInit(templatesInvolved, bag.name, () => new Set()).add(rb.recipe.template_id);
254264
});
265+
this.debug(bag.name, "recipes involved", recipesInvolved.size);
266+
this.debug(bag.name, "templates involved", templatesInvolved.size);
255267
});
256268

257269
const recipesToEdit = Array.from(new Set(Array.from(recipesInvolved.values(), e => [...e]).flat()));
@@ -294,6 +306,8 @@ export class BagImportWriter extends PerClassImportWriter<"bag"> {
294306
});
295307
}
296308

309+
super.rename(renames);
310+
297311
}
298312

299313
async upsert(bags: UpsertBagInput[]) {

packages/mws/src/services/cache.ts renamed to packages/mws/src/new-managers/plugin-cache.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { TW } from "tiddlywiki";
66
import { createGzip, gzipSync } from "zlib";
77
import { awaitPipe, BodyFormat, checkPath, checkQueryKeys, dist_resolve, SendFileOptions, ServerRequest } from "@tiddlywiki/server";
88
import { serverEvents } from "@tiddlywiki/events";
9-
import { mapGetInit } from "../new-managers";
9+
import { mapGetInit } from ".";
1010
import { open, stat } from "fs/promises";
1111
import { PassThrough, Readable } from "stream";
1212
import { pipeline } from "stream/promises";
Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ export class SessionManager {
177177
login1 = zodSession("/login/1", z => z.object({
178178
username: z.prismaField("Users", "username", "string"),
179179
startLoginRequest: z.string(),
180-
}), async () => { }, async (state, prisma) => {
180+
}), async (state) => { state.assertRefererPrefix(["/login"]); }, async (state, prisma) => {
181181
const { username, startLoginRequest } = state.data;
182182

183183
const user = await prisma.users.findUnique({
@@ -209,7 +209,7 @@ export class SessionManager {
209209
finishLoginRequest: z.string(),
210210
loginSession: z.string(),
211211
skipCookie: z.boolean().optional().default(false),
212-
}), async () => { }, async (state, prisma) => {
212+
}), async (state) => { state.assertRefererPrefix(["/login"]); }, async (state, prisma) => {
213213
const { finishLoginRequest, skipCookie, loginSession } = state.data;
214214

215215
if (!loginSession) throw "Login session not found.";
@@ -284,6 +284,7 @@ export class SessionManager {
284284
emailOrUsername: z.string(),
285285
resetCode: z.string().optional(),
286286
}), async state => {
287+
state.assertRefererPrefix(["/login"]);
287288
if (state.data.resetCode)
288289
await passwordLookupRateLimiter.wait(state, "");
289290
}, async (state, prisma) => {
@@ -307,7 +308,7 @@ export class SessionManager {
307308
user_id: z.string(),
308309
registrationRequest: z.string().optional(),
309310
registrationRecord: z.string().optional(),
310-
}), async () => { }, async (state, prisma) => {
311+
}), async (state) => { state.assertRefererPrefix(["/login"]); }, async (state, prisma) => {
311312
const user = await prisma.users.findUnique({ where: { user_id: state.data.user_id, }, });
312313
if (!user)
313314
throw new SendError("RECORD_KEY_NOT_FOUND", 400, { table: "users", name: "user_id" });
@@ -348,7 +349,10 @@ export class SessionManager {
348349
session_id: z.string().optional(),
349350
signature: z.string().optional(),
350351
}),
351-
async (state) => { state.okUser(); },
352+
async (state) => {
353+
state.okUser();
354+
state.assertRefererPrefix(["/profile"]);
355+
},
352356
async (state, prisma) => {
353357

354358
const { user_id, registrationRecord, registrationRequest } = state.data;

0 commit comments

Comments
 (0)