Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions migrations/20260508120000_add_access_groups.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import type { Knex } from "knex";

export async function up(knex: Knex): Promise<void> {
// Access groups define visibility scopes for pages.
// "public" = visible to everyone without login.
// "admin" = grants access to all pages (assigned to admin role).
if (!(await knex.schema.hasTable("access_groups"))) {
await knex.schema.createTable("access_groups", (table) => {
table.string("id", 100).primary();
table.text("group_name").notNullable();
table.text("description");
table.integer("is_system").notNullable().defaultTo(0);
table.timestamp("created_at").defaultTo(knex.fn.now());
table.timestamp("updated_at").defaultTo(knex.fn.now());
});
}

// Junction: which access groups are assigned to which page
if (!(await knex.schema.hasTable("page_access_groups"))) {
await knex.schema.createTable("page_access_groups", (table) => {
table.integer("page_id").unsigned().notNullable()
.references("id").inTable("pages").onDelete("CASCADE");
table.string("access_group_id", 100).notNullable()
.references("id").inTable("access_groups").onDelete("CASCADE");
table.timestamp("created_at").defaultTo(knex.fn.now());

table.primary(["page_id", "access_group_id"]);
});
}

// Junction: which access groups a role can see
if (!(await knex.schema.hasTable("role_access_groups"))) {
await knex.schema.createTable("role_access_groups", (table) => {
table.string("role_id", 100).notNullable()
.references("id").inTable("roles").onDelete("CASCADE");
table.string("access_group_id", 100).notNullable()
.references("id").inTable("access_groups").onDelete("CASCADE");
table.timestamp("created_at").defaultTo(knex.fn.now());

table.primary(["role_id", "access_group_id"]);
});
}

// Seed the "public" system group
const publicExists = await knex("access_groups").where("id", "public").first();
if (!publicExists) {
await knex("access_groups").insert({
id: "public",
group_name: "Public",
description: "Pages with this group are visible to everyone without login",
is_system: 1,
created_at: knex.fn.now(),
updated_at: knex.fn.now(),
});
}

// Seed the "admin" system group
const adminExists = await knex("access_groups").where("id", "admin").first();
if (!adminExists) {
await knex("access_groups").insert({
id: "admin",
group_name: "Admin",
description: "Grants access to all pages regardless of their groups",
is_system: 1,
created_at: knex.fn.now(),
updated_at: knex.fn.now(),
});
}

// Assign "public" to all existing pages (backward compatibility)
const allPages = await knex("pages").select("id");
for (const page of allPages) {
const exists = await knex("page_access_groups")
.where({ page_id: page.id, access_group_id: "public" })
.first();
if (!exists) {
await knex("page_access_groups").insert({
page_id: page.id,
access_group_id: "public",
created_at: knex.fn.now(),
});
}
}

// Assign "admin" group to the admin role
const adminRoleExists = await knex("roles").where("id", "admin").first();
if (adminRoleExists) {
const alreadyAssigned = await knex("role_access_groups")
.where({ role_id: "admin", access_group_id: "admin" })
.first();
if (!alreadyAssigned) {
await knex("role_access_groups").insert({
role_id: "admin",
access_group_id: "admin",
created_at: knex.fn.now(),
});
}
}
}

export async function down(knex: Knex): Promise<void> {
await knex.schema.dropTableIfExists("role_access_groups");
await knex.schema.dropTableIfExists("page_access_groups");
await knex.schema.dropTableIfExists("access_groups");
}
9 changes: 9 additions & 0 deletions src/lib/allPerms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,15 @@ export const ACTION_PERMISSION_MAP: Record<string, string | null> = {
updateRolePermissions: "roles.assign_permissions",
addUserToRole: "roles.assign_users",
removeUserFromRole: "roles.assign_users",

// Access Groups
getAccessGroups: "pages.read",
createAccessGroup: "roles.write",
deleteAccessGroup: "roles.write",
Comment on lines +227 to +228

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Require page-write permission when deleting access groups.

deleteAccessGroup can cascade-remove page_access_groups, so roles.write alone lets a role manager change page visibility. Add a pages.write check for this action, or require both roles.write and pages.write in the dispatcher.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/allPerms.ts` around lines 227 - 228, The deleteAccessGroup permission
mapping currently only uses roles.write, which allows cascading changes to
page_access_groups without page-level authorization. Update the permission
configuration in allPerms so deleteAccessGroup also requires pages.write, or
adjust the dispatcher to enforce both roles.write and pages.write when handling
this action. Use the deleteAccessGroup mapping in allPerms as the place to
locate and fix the check.

getPageAccessGroups: "pages.read",
setPageAccessGroups: "pages.write",
getRoleAccessGroups: "roles.read",
setRoleAccessGroups: "roles.write",
};

export const ROUTE_PERMISSION_MAP: Record<string, string | null> = {
Expand Down
21 changes: 12 additions & 9 deletions src/lib/server/api-server/pages/get.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,34 @@
import { json } from "@sveltejs/kit";
import type { APIServerRequest } from "$lib/server/types/api-server";
import { GetAllPages } from "$lib/server/controllers/pagesController";
import { GetAccessiblePages } from "$lib/server/controllers/pagesController";
import { GetSiteDataByKey } from "$lib/server/controllers/siteDataController";
import { GetLoggedInSession } from "$lib/server/controllers/controller";
import type { PageNavItem } from "$lib/server/controllers/dashboardController";
import type { PageOrderingSettings } from "$lib/types/site";

/**
* GET /dashboard-apis/pages
* Returns all pages as PageNavItem[] (page_title, page_path)
* Respects pageOrderingSettings if enabled
* Returns pages the current user is allowed to see, respecting
* access groups and page ordering settings.
*/
export default async function get(_req: APIServerRequest): Promise<Response> {
const allPagesData = await GetAllPages();
export default async function get(req: APIServerRequest): Promise<Response> {
// Get the logged-in user from the session cookie (null if anonymous)
const user = req.cookies ? await GetLoggedInSession(req.cookies) : null;

// Get only pages the user can access
const accessiblePages = await GetAccessiblePages(user);
const pageOrderingSettings = (await GetSiteDataByKey("pageOrderingSettings")) as PageOrderingSettings | null;

let orderedPages = allPagesData;
let orderedPages = accessiblePages;

if (pageOrderingSettings?.enabled && pageOrderingSettings.order?.length > 0) {
const orderMap = new Map(pageOrderingSettings.order.map((id, idx) => [id, idx]));
orderedPages = [...allPagesData].sort((a, b) => {
orderedPages = [...accessiblePages].sort((a, b) => {
const aIdx = orderMap.get(a.id);
const bIdx = orderMap.get(b.id);
// Pages in the order list come first, sorted by their position
if (aIdx !== undefined && bIdx !== undefined) return aIdx - bIdx;
if (aIdx !== undefined) return -1;
if (bIdx !== undefined) return 1;
// Pages not in the order list keep their default order (by id)
return a.id - b.id;
});
}
Expand Down
198 changes: 194 additions & 4 deletions src/lib/server/controllers/pagesController.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import db from "../db/db.js";
import type { PageRecord, PageRecordInsert, PageMonitorRecord, PageMonitorRecordInsert } from "../types/db.js";
import type {
PageRecord,
PageRecordInsert,
PageMonitorRecord,
PageMonitorRecordInsert,
UserRecordPublic,
} from "../types/db.js";
import { GetSiteDataByKey } from "./siteDataController.js";

// ============ Page CRUD Operations ============

/**
* Create a new page
* Create a new page.
* When the site setting "autoPublicPages" is true (default),
* newly created pages automatically get the "public" access group.
*/
export async function CreatePage(data: PageRecordInsert): Promise<PageRecord> {
// Validate required fields
if (data.page_path === undefined || data.page_path === null || !data.page_title || !data.page_header) {
throw new Error("page_path, page_title, and page_header are required");
}

// Make page_path URL-friendly: lowercase, replace spaces with hyphens, remove special chars (including leading slashes)
// Make page_path URL-friendly
data.page_path = data.page_path
.toLowerCase()
.trim()
Expand All @@ -25,7 +34,15 @@ export async function CreatePage(data: PageRecordInsert): Promise<PageRecord> {
throw new Error(`Page with path "${data.page_path}" already exists`);
}

return await db.createPage(data);
const newPage = await db.createPage(data);

// Auto-assign "public" group to new pages (unless disabled in settings)
const autoPublic = await GetSiteDataByKey("autoPublicPages");
if (autoPublic !== false && autoPublic !== "false") {
await db.setPageAccessGroups(newPage.id, ["public"]);
Comment on lines +37 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep page creation and default access assignment atomic.

If setPageAccessGroups fails after createPage, the request errors but the page remains created without its intended visibility group; retrying then hits the duplicate path check. Wrap both writes in one transaction or compensate by deleting the new page before rethrowing.

Localized compensation option
   const newPage = await db.createPage(data);
 
-  // Auto-assign "public" group to new pages (unless disabled in settings)
-  const autoPublic = await GetSiteDataByKey("autoPublicPages");
-  if (autoPublic !== false && autoPublic !== "false") {
-    await db.setPageAccessGroups(newPage.id, ["public"]);
+  try {
+    // Auto-assign "public" group to new pages (unless disabled in settings)
+    const autoPublic = await GetSiteDataByKey("autoPublicPages");
+    if (autoPublic !== false && autoPublic !== "false") {
+      await db.setPageAccessGroups(newPage.id, ["public"]);
+    }
+  } catch (error) {
+    await db.deletePage(newPage.id);
+    throw error;
   }
 
   return newPage;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const newPage = await db.createPage(data);
// Auto-assign "public" group to new pages (unless disabled in settings)
const autoPublic = await GetSiteDataByKey("autoPublicPages");
if (autoPublic !== false && autoPublic !== "false") {
await db.setPageAccessGroups(newPage.id, ["public"]);
const newPage = await db.createPage(data);
try {
// Auto-assign "public" group to new pages (unless disabled in settings)
const autoPublic = await GetSiteDataByKey("autoPublicPages");
if (autoPublic !== false && autoPublic !== "false") {
await db.setPageAccessGroups(newPage.id, ["public"]);
}
} catch (error) {
await db.deletePage(newPage.id);
throw error;
}
return newPage;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/server/controllers/pagesController.ts` around lines 37 - 42, The page
creation flow in pagesController’s createPage path is not atomic because
db.createPage succeeds before db.setPageAccessGroups, so a failure can leave a
page created without its default visibility group. Fix this by making both
writes part of one transaction in the same controller flow, or by adding
compensation to delete the newly created page and rethrow if setPageAccessGroups
fails. Use the createPage, setPageAccessGroups, and GetSiteDataByKey logic to
locate the affected block.

}

return newPage;
}

/**
Expand Down Expand Up @@ -228,3 +245,176 @@ export async function GetPageByPathWithMonitors(
const monitors = await db.getPageMonitorsExcludeHidden(page.id);
return { page, monitors };
}

// ============ Access Group Operations ============

// Reserved group ID: role with "admin" can see all pages
const ADMIN_GROUP = "admin";
// Reserved group ID: pages with "public" are visible without login
const PUBLIC_GROUP = "public";

/**
* Check if a user can access a specific page.
*
* Rules (evaluated top to bottom, first match wins):
* 1. Page has "public" group → allow (no login needed)
* 2. No login → redirect to sign-in
* 3. User's roles include the "admin" group → allow (admin sees everything)
* 4. Page has no access groups → denied (unconfigured pages are hidden)
* 5. Any overlap between page groups and role groups → allow
* 6. No overlap → denied
*
* Returns: "allow" | "login_required" | "denied"
*/
export async function CheckPageAccess(
page_id: number,
user: UserRecordPublic | null,
): Promise<"allow" | "login_required" | "denied"> {
const pageGroups = await db.getAccessGroupsForPage(page_id);

// Rule 1: public pages are visible to everyone
if (pageGroups.includes(PUBLIC_GROUP)) {
return "allow";
}

// Rule 2: non-public pages require login
if (!user) {
return "login_required";
}

// Rule 3: admin group grants access to all pages
const roleGroups = await db.getAccessGroupsForRoles(user.role_ids);
if (roleGroups.includes(ADMIN_GROUP)) {
return "allow";
}

// Rule 4: pages with no groups are hidden (except for admin)
if (pageGroups.length === 0) {
return "denied";
}

// Rule 5+6: check for overlap between page and role groups
const hasAccess = pageGroups.some((pg) => roleGroups.includes(pg));
return hasAccess ? "allow" : "denied";
}

/**
* Get all pages that a user is allowed to see.
* Used by the page switcher to filter the dropdown.
*/
export async function GetAccessiblePages(
user: UserRecordPublic | null,
): Promise<PageRecord[]> {
const allPages = await db.getAllPages();
if (allPages.length === 0) return [];

// Load access groups for all pages in one query
const pageIds = allPages.map((p) => p.id);
const pageGroupsMap = await db.getAccessGroupsForPages(pageIds);

// Load user's role groups (if logged in)
let roleGroups: string[] = [];
let isAdmin = false;
if (user) {
roleGroups = await db.getAccessGroupsForRoles(user.role_ids);
isAdmin = roleGroups.includes(ADMIN_GROUP);
}

return allPages.filter((page) => {
const groups = pageGroupsMap.get(page.id) || [];

// Public pages are visible to everyone
if (groups.includes(PUBLIC_GROUP)) return true;

// Non-public pages require login
if (!user) return false;

// Admin sees everything
if (isAdmin) return true;

// Pages with no groups are hidden
if (groups.length === 0) return false;

// Check if role groups overlap with page groups
return groups.some((g) => roleGroups.includes(g));
});
}

// ============ Access Group Admin Operations ============

/**
* Get all access groups (for admin UI dropdowns)
*/
export async function GetAllAccessGroups() {
return await db.getAllAccessGroups();
}

/**
* Create a new access group
*/
export async function CreateAccessGroup(data: { id: string; group_name: string; description?: string }) {
Comment on lines +351 to +355

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Only "public" is explicitly reserved — "admin" is not guarded

The function blocks creation of a group named "public" but not "admin". Both are system groups with special semantics. If the seeded "admin" row were absent (manual DB cleanup, failed migration), a caller could create a non-system "admin" group (is_system = 0), which is then deletable through the normal delete path, silently stripping admin-level page access from all roles. The guard should also reject data.id === "admin" with a matching error.

if (!data.id || !data.group_name) {
throw new Error("id and group_name are required");
}

// Normalize ID: lowercase, hyphens, no special chars
data.id = data.id.toLowerCase().trim().replace(/\s+/g, "-").replace(/[^a-z0-9_-]/g, "");

if (data.id === "public") {
throw new Error("Cannot create a group with the reserved ID 'public'");
}
Comment on lines +360 to +365

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Revalidate normalized IDs and reserve admin.

An ID like "!!!" normalizes to "" after the required-field check, and admin is also reserved by the access logic as a global override group. Reject both after normalization.

Suggested fix
   // Normalize ID: lowercase, hyphens, no special chars
   data.id = data.id.toLowerCase().trim().replace(/\s+/g, "-").replace(/[^a-z0-9_-]/g, "");
 
-  if (data.id === "public") {
-    throw new Error("Cannot create a group with the reserved ID 'public'");
+  if (!data.id) {
+    throw new Error("id must contain at least one URL-safe character");
+  }
+
+  if (data.id === PUBLIC_GROUP || data.id === ADMIN_GROUP) {
+    throw new Error(`Cannot create a group with the reserved ID '${data.id}'`);
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/server/controllers/pagesController.ts` around lines 360 - 365, After
normalizing the group ID in pagesController, revalidate the resulting data.id
before proceeding because values like "!!!" can normalize to an empty string;
also treat "admin" as a reserved ID alongside "public" and reject both in the
same validation path. Update the existing ID normalization/validation logic
around the data.id assignment so the create-group flow fails fast for empty
normalized IDs and reserved names.


const existing = (await db.getAllAccessGroups()).find((g) => g.id === data.id);
if (existing) {
throw new Error(`Access group '${data.id}' already exists`);
}

await db.createAccessGroup(data);
return { success: true, id: data.id };
}

/**
* Delete an access group. System groups (public, admin) cannot be deleted.
*/
export async function DeleteAccessGroup(id: string) {
const allGroups = await db.getAllAccessGroups();
const group = allGroups.find((g) => g.id === id);
if (!group) {
throw new Error(`Access group '${id}' not found`);
}
if ((group as any).is_system === 1) {
throw new Error(`Cannot delete system group '${id}'`);
}
await db.deleteAccessGroup(id);
return { success: true };
}

/**
* Get access groups assigned to a page
*/
export async function GetPageAccessGroups(page_id: number): Promise<string[]> {
return await db.getAccessGroupsForPage(page_id);
}

/**
* Set access groups for a page (replaces all existing assignments)
*/
export async function SetPageAccessGroups(page_id: number, group_ids: string[]) {
await db.setPageAccessGroups(page_id, group_ids);
return { success: true };
}

/**
* Get access groups assigned to a role
*/
export async function GetRoleAccessGroups(role_id: string): Promise<string[]> {
return await db.getAccessGroupsForRole(role_id);
}

/**
* Set access groups for a role (replaces all existing assignments)
*/
export async function SetRoleAccessGroups(role_id: string, group_ids: string[]) {
await db.setRoleAccessGroups(role_id, group_ids);
Comment on lines +402 to +418

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Validate assignment group IDs before writing.

The manage API passes group_ids directly here; invalid IDs become dialect-specific FK errors, and duplicates can violate the composite primary key. De-duplicate and verify IDs against getAllAccessGroups() before calling the repository.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/server/controllers/pagesController.ts` around lines 402 - 418,
Validate the incoming group_ids in SetPageAccessGroups and SetRoleAccessGroups
before calling db.setPageAccessGroups or db.setRoleAccessGroups. De-duplicate
the array, fetch the allowed IDs via getAllAccessGroups(), and reject any
unknown IDs so the controller never writes invalid or duplicate assignments that
can trigger FK or composite-key errors.

return { success: true };
}
Loading