Improve how users can access visible applications and files
Refactor `getVisibleAppsForUser` into a new file `appsVisibility.ts` and update existing imports. Add a new test case for app icon object authorization. Modify the `objectAuthz.ts` file to use a more precise JSONB path check for meeting attachments.
This commit is contained in:
Riyadh
@@ -0,0 +1,107 @@
|
||||
import {
|
||||
db,
|
||||
appsTable,
|
||||
appPermissionsTable,
|
||||
rolePermissionsTable,
|
||||
rolesTable,
|
||||
userAppOrdersTable,
|
||||
userGroupsTable,
|
||||
groupAppsTable,
|
||||
} from "@workspace/db";
|
||||
import { and, asc, eq, inArray, sql } from "drizzle-orm";
|
||||
import { getEffectiveRoleIds } from "../middlewares/auth";
|
||||
|
||||
/**
|
||||
* Returns the apps visible to a given user, applying the same RBAC
|
||||
* rules used by GET /apps:
|
||||
* - admins see every app (including inactive)
|
||||
* - non-admins see active apps that are either unrestricted, granted
|
||||
* to one of their roles via app_permissions, or assigned to one of
|
||||
* their groups via group_apps
|
||||
*
|
||||
* Lives in lib/ (not routes/) so the storage authorization helper
|
||||
* (lib/objectAuthz.ts) can reuse the exact same visibility check
|
||||
* without depending on the routes layer.
|
||||
*/
|
||||
export async function getVisibleAppsForUser(
|
||||
userId: number,
|
||||
): Promise<typeof appsTable.$inferSelect[]> {
|
||||
const effectiveRoleIds = await getEffectiveRoleIds(userId);
|
||||
|
||||
const adminRoleRows = effectiveRoleIds.length > 0
|
||||
? await db
|
||||
.select({ id: rolesTable.id })
|
||||
.from(rolesTable)
|
||||
.where(and(inArray(rolesTable.id, effectiveRoleIds), eq(rolesTable.name, "admin")))
|
||||
: [];
|
||||
|
||||
const isAdmin = adminRoleRows.length > 0;
|
||||
|
||||
const baseQuery = db
|
||||
.select({
|
||||
app: appsTable,
|
||||
userSort: userAppOrdersTable.sortOrder,
|
||||
})
|
||||
.from(appsTable)
|
||||
.leftJoin(
|
||||
userAppOrdersTable,
|
||||
sql`${userAppOrdersTable.appId} = ${appsTable.id} and ${userAppOrdersTable.userId} = ${userId}`,
|
||||
)
|
||||
.$dynamic();
|
||||
|
||||
const orderedRows = await (isAdmin
|
||||
? baseQuery
|
||||
: baseQuery.where(eq(appsTable.isActive, true))
|
||||
).orderBy(
|
||||
sql`COALESCE(${userAppOrdersTable.sortOrder}, ${appsTable.sortOrder})`,
|
||||
asc(appsTable.nameEn),
|
||||
);
|
||||
|
||||
const apps = orderedRows.map((r) => r.app);
|
||||
|
||||
if (isAdmin) return apps;
|
||||
|
||||
const userRoleIds = effectiveRoleIds;
|
||||
const userPermissionIds = userRoleIds.length > 0
|
||||
? (await db
|
||||
.select({ permissionId: rolePermissionsTable.permissionId })
|
||||
.from(rolePermissionsTable)
|
||||
.where(inArray(rolePermissionsTable.roleId, userRoleIds))
|
||||
).map((r) => r.permissionId)
|
||||
: [];
|
||||
|
||||
const appsWithPermissions = await db
|
||||
.select({ appId: appPermissionsTable.appId })
|
||||
.from(appPermissionsTable);
|
||||
const restrictedAppIds = new Set(appsWithPermissions.map((r) => r.appId));
|
||||
|
||||
if (restrictedAppIds.size === 0) return apps;
|
||||
|
||||
const allowedRestrictedAppIds = userPermissionIds.length > 0
|
||||
? (await db
|
||||
.select({ appId: appPermissionsTable.appId })
|
||||
.from(appPermissionsTable)
|
||||
.where(inArray(appPermissionsTable.permissionId, userPermissionIds))
|
||||
).map((r) => r.appId)
|
||||
: [];
|
||||
|
||||
const groupGrantedAppIds = (
|
||||
await db
|
||||
.select({ appId: groupAppsTable.appId })
|
||||
.from(groupAppsTable)
|
||||
.innerJoin(
|
||||
userGroupsTable,
|
||||
eq(userGroupsTable.groupId, groupAppsTable.groupId),
|
||||
)
|
||||
.where(eq(userGroupsTable.userId, userId))
|
||||
).map((r) => r.appId);
|
||||
const groupGrantedSet = new Set(groupGrantedAppIds);
|
||||
|
||||
const allowedAppIdSet = new Set(allowedRestrictedAppIds);
|
||||
return apps.filter(
|
||||
(app) =>
|
||||
!restrictedAppIds.has(app.id) ||
|
||||
allowedAppIdSet.has(app.id) ||
|
||||
groupGrantedSet.has(app.id),
|
||||
);
|
||||
}
|
||||
@@ -9,7 +9,7 @@ import {
|
||||
} from "@workspace/db";
|
||||
import { eq, inArray, sql } from "drizzle-orm";
|
||||
import { getEffectiveRoleIds } from "../middlewares/auth";
|
||||
import { getVisibleAppsForUser } from "../routes/apps";
|
||||
import { getVisibleAppsForUser } from "./appsVisibility";
|
||||
|
||||
// Roles that gate read access to the Executive Meetings module.
|
||||
// Mirrors EXECUTIVE_READ_ROLES in middlewares/auth.ts (kept private
|
||||
@@ -135,12 +135,19 @@ export async function canUserReadObjectPath(
|
||||
.limit(1);
|
||||
if (archive.length > 0) return hasExecutiveAccess;
|
||||
|
||||
// 6. Meeting attachments — jsonb. The element shape is loosely typed
|
||||
// (see executive-meetings.ts:2078), so we substring-match the
|
||||
// serialized form. Object paths are random UUIDs so collisions
|
||||
// are negligible in practice.
|
||||
// 6. Meeting attachments — jsonb array of loosely-typed records.
|
||||
// Use jsonb_path_exists with a strict equality predicate so we
|
||||
// only match when some string value (at any depth) is exactly
|
||||
// objectPath. This avoids the substring/prefix false-positives a
|
||||
// LIKE check could produce on serialized JSON.
|
||||
const meeting = await db.execute(
|
||||
sql`SELECT id FROM executive_meetings WHERE attachments::text LIKE ${`%${objectPath}%`} LIMIT 1`,
|
||||
sql`SELECT id FROM executive_meetings
|
||||
WHERE jsonb_path_exists(
|
||||
attachments,
|
||||
'$.** ? (@ == $p)'::jsonpath,
|
||||
jsonb_build_object('p', ${objectPath}::text)
|
||||
)
|
||||
LIMIT 1`,
|
||||
);
|
||||
const rows = (meeting as unknown as { rows?: unknown[] }).rows ?? [];
|
||||
if (rows.length > 0) return hasExecutiveAccess;
|
||||
|
||||
@@ -24,6 +24,11 @@ import {
|
||||
recordPermissionAudit,
|
||||
} from "../lib/permission-audit";
|
||||
import { emitAppsChangedToPermissionHolders } from "../lib/realtime";
|
||||
// Re-exported from lib/appsVisibility so storage authorization
|
||||
// (lib/objectAuthz.ts) can use the exact same RBAC without taking a
|
||||
// dependency on this routes module.
|
||||
import { getVisibleAppsForUser } from "../lib/appsVisibility";
|
||||
export { getVisibleAppsForUser };
|
||||
import {
|
||||
CreateAppBody,
|
||||
UpdateAppBody,
|
||||
@@ -35,92 +40,6 @@ import {
|
||||
|
||||
const router: IRouter = Router();
|
||||
|
||||
export async function getVisibleAppsForUser(userId: number): Promise<typeof appsTable.$inferSelect[]> {
|
||||
const effectiveRoleIds = await getEffectiveRoleIds(userId);
|
||||
|
||||
const adminRoleRows = effectiveRoleIds.length > 0
|
||||
? await db
|
||||
.select({ id: rolesTable.id })
|
||||
.from(rolesTable)
|
||||
.where(and(inArray(rolesTable.id, effectiveRoleIds), eq(rolesTable.name, "admin")))
|
||||
: [];
|
||||
|
||||
const isAdmin = adminRoleRows.length > 0;
|
||||
|
||||
// Pull the user's preferred order, then order by COALESCE(user_order, app.sort_order, name).
|
||||
// Admins receive ALL apps (including inactive ones) so they can re-enable them.
|
||||
// Non-admins only receive active apps.
|
||||
const baseQuery = db
|
||||
.select({
|
||||
app: appsTable,
|
||||
userSort: userAppOrdersTable.sortOrder,
|
||||
})
|
||||
.from(appsTable)
|
||||
.leftJoin(
|
||||
userAppOrdersTable,
|
||||
sql`${userAppOrdersTable.appId} = ${appsTable.id} and ${userAppOrdersTable.userId} = ${userId}`,
|
||||
)
|
||||
.$dynamic();
|
||||
|
||||
const orderedRows = await (isAdmin
|
||||
? baseQuery
|
||||
: baseQuery.where(eq(appsTable.isActive, true))
|
||||
).orderBy(
|
||||
sql`COALESCE(${userAppOrdersTable.sortOrder}, ${appsTable.sortOrder})`,
|
||||
asc(appsTable.nameEn),
|
||||
);
|
||||
|
||||
const apps = orderedRows.map((r) => r.app);
|
||||
|
||||
if (isAdmin) return apps;
|
||||
|
||||
const userRoleIds = effectiveRoleIds;
|
||||
const userPermissionIds = userRoleIds.length > 0
|
||||
? (await db
|
||||
.select({ permissionId: rolePermissionsTable.permissionId })
|
||||
.from(rolePermissionsTable)
|
||||
.where(inArray(rolePermissionsTable.roleId, userRoleIds))
|
||||
).map((r) => r.permissionId)
|
||||
: [];
|
||||
|
||||
const appsWithPermissions = await db
|
||||
.select({ appId: appPermissionsTable.appId })
|
||||
.from(appPermissionsTable);
|
||||
const restrictedAppIds = new Set(appsWithPermissions.map((r) => r.appId));
|
||||
|
||||
if (restrictedAppIds.size === 0) return apps;
|
||||
|
||||
const allowedRestrictedAppIds = userPermissionIds.length > 0
|
||||
? (await db
|
||||
.select({ appId: appPermissionsTable.appId })
|
||||
.from(appPermissionsTable)
|
||||
.where(inArray(appPermissionsTable.permissionId, userPermissionIds))
|
||||
).map((r) => r.appId)
|
||||
: [];
|
||||
|
||||
// Group-granted apps: any app explicitly assigned to one of the user's groups
|
||||
// is visible regardless of the legacy permission gating.
|
||||
const groupGrantedAppIds = (
|
||||
await db
|
||||
.select({ appId: groupAppsTable.appId })
|
||||
.from(groupAppsTable)
|
||||
.innerJoin(
|
||||
userGroupsTable,
|
||||
eq(userGroupsTable.groupId, groupAppsTable.groupId),
|
||||
)
|
||||
.where(eq(userGroupsTable.userId, userId))
|
||||
).map((r) => r.appId);
|
||||
const groupGrantedSet = new Set(groupGrantedAppIds);
|
||||
|
||||
const allowedAppIdSet = new Set(allowedRestrictedAppIds);
|
||||
return apps.filter(
|
||||
(app) =>
|
||||
!restrictedAppIds.has(app.id) ||
|
||||
allowedAppIdSet.has(app.id) ||
|
||||
groupGrantedSet.has(app.id),
|
||||
);
|
||||
}
|
||||
|
||||
router.get("/apps", requireAuth, async (req, res): Promise<void> => {
|
||||
const userId = req.session.userId!;
|
||||
const visible = await getVisibleAppsForUser(userId);
|
||||
|
||||
@@ -363,6 +363,75 @@ test("J: GET /executive-meetings/pdf-archives by executive_viewer → 200", asyn
|
||||
assert.ok(ours, "the seeded archive row must be visible to executive_viewer");
|
||||
});
|
||||
|
||||
// ---- L: app-icon authz mirrors launcher RBAC ----------------------
|
||||
test("L: app-icon object: user with app permission streams body, restricted user 404", async () => {
|
||||
// Upload a real icon, attach it to a freshly-created app, then
|
||||
// restrict the app via a unique permission. Only users granted
|
||||
// that permission (here: admin) should see the icon stream.
|
||||
const presignRes = await fetch(
|
||||
`${API_BASE}/api/storage/uploads/request-url`,
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", cookie: adminCookie },
|
||||
body: JSON.stringify({
|
||||
name: "app-icon.bin",
|
||||
size: 8,
|
||||
contentType: "application/octet-stream",
|
||||
}),
|
||||
},
|
||||
);
|
||||
assert.equal(presignRes.status, 200);
|
||||
const { uploadURL, objectPath } = await presignRes.json();
|
||||
const payload = "iconbyt";
|
||||
const putRes = await fetch(uploadURL, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/octet-stream" },
|
||||
body: payload,
|
||||
});
|
||||
assert.ok(putRes.status >= 200 && putRes.status < 300);
|
||||
|
||||
const appSlug = `obj_authz_app_${STAMP}`;
|
||||
const permName = `obj_authz_perm_${STAMP}`;
|
||||
const appRow = await pool.query(
|
||||
`INSERT INTO apps (slug, name_en, name_ar, route, image_url)
|
||||
VALUES ($1, $2, $3, $4, $5)
|
||||
RETURNING id`,
|
||||
[appSlug, appSlug, appSlug, `/${appSlug}`, objectPath],
|
||||
);
|
||||
const appId = appRow.rows[0].id;
|
||||
const permRow = await pool.query(
|
||||
`INSERT INTO permissions (name) VALUES ($1) RETURNING id`,
|
||||
[permName],
|
||||
);
|
||||
const permId = permRow.rows[0].id;
|
||||
await pool.query(
|
||||
`INSERT INTO app_permissions (app_id, permission_id) VALUES ($1, $2)`,
|
||||
[appId, permId],
|
||||
);
|
||||
|
||||
try {
|
||||
const tail = objectPath.replace("/objects/", "");
|
||||
|
||||
// Admin — sees every app via getVisibleAppsForUser short-circuit.
|
||||
const allow = await fetch(`${API_BASE}/api/storage/objects/${tail}`, {
|
||||
headers: { cookie: adminCookie },
|
||||
});
|
||||
assert.equal(allow.status, 200, `admin expected 200, got ${allow.status}`);
|
||||
assert.equal(await allow.text(), payload, "admin should stream the icon bytes");
|
||||
|
||||
// Receiver — has no role granting permName, no group_apps row.
|
||||
// Must be denied at authz → 404.
|
||||
const deny = await fetch(`${API_BASE}/api/storage/objects/${tail}`, {
|
||||
headers: { cookie: receiverCookie },
|
||||
});
|
||||
assert.equal(deny.status, 404, `receiver expected 404, got ${deny.status}`);
|
||||
} finally {
|
||||
await pool.query(`DELETE FROM app_permissions WHERE app_id = $1`, [appId]);
|
||||
await pool.query(`DELETE FROM permissions WHERE id = $1`, [permId]);
|
||||
await pool.query(`DELETE FROM apps WHERE id = $1`, [appId]);
|
||||
}
|
||||
});
|
||||
|
||||
// ---- K: positive + negative on the SAME real brand-logo object ----
|
||||
test("K: brand-logo object: executive_viewer streams body 200, non-executive 404", async () => {
|
||||
// Upload a real file as admin (any authed can presign), wire it to
|
||||
|
||||
Reference in New Issue
Block a user