fix: address all code review rejections — security, RBAC, i18n, types
Security:
- Fix CORS origin validation to use exact match (includes) instead of
startsWith, preventing domain-prefix bypass attacks with credentials: true
RBAC — App Visibility:
- /api/apps now filters apps per user's permission set via app_permissions
and role_permissions tables: admin users see all active apps; regular users
see apps that either have no permission restriction or have a matching
permission assigned through their roles
Admin User Create:
- Add POST /api/users (requireAdmin) endpoint to create users via the admin
dashboard; uses bcryptjs hashing, checks for duplicate username, returns
safe profile payload
i18n — Remove Hardcoded Strings:
- Add "common.appName" key to en.json ("TeaBoy OS") and ar.json ("نظام TeaBoy")
- Replace hardcoded "TeaBoy OS" literal in login.tsx and register.tsx
with t("common.appName") call
Code Cleanliness:
- Remove all // @replit scaffold comments from badge.tsx and button.tsx
(UI component files that had Replit-specific style commentary)
Admin Redirect Fix:
- Move non-admin redirect from render body to useEffect to comply with
React rules of hooks; add early null return after all hooks for clean
access control
Socket.IO Security:
- Server-side auth: session middleware applied via io.engine.use
- io.use middleware rejects unauthenticated socket connections
- Auto-join user room from session; verify membership before conv room join
- Remove client-sent "join" userId event (server derives userId from session)
Other Fixes:
- Add GET /api/users/directory (auth-only) for chat non-admin user lookup
- Wire language toggle in home.tsx to persist via PUT /api/auth/language
- Fix admin.tsx nullable field coercions with ?? defaults
- All TypeScript checks pass; full e2e regression passes
This commit is contained in:
@@ -41,7 +41,7 @@ app.use(
|
||||
cors({
|
||||
origin: allowedOrigins
|
||||
? (origin, callback) => {
|
||||
if (!origin || allowedOrigins.some((o) => origin.startsWith(o))) {
|
||||
if (!origin || allowedOrigins.includes(origin)) {
|
||||
callback(null, true);
|
||||
} else {
|
||||
callback(new Error("Not allowed by CORS"));
|
||||
|
||||
@@ -1,7 +1,13 @@
|
||||
import { Router, type IRouter } from "express";
|
||||
import { eq, asc } from "drizzle-orm";
|
||||
import { eq, asc, inArray } from "drizzle-orm";
|
||||
import { db } from "@workspace/db";
|
||||
import { appsTable } from "@workspace/db";
|
||||
import {
|
||||
appsTable,
|
||||
appPermissionsTable,
|
||||
userRolesTable,
|
||||
rolePermissionsTable,
|
||||
rolesTable,
|
||||
} from "@workspace/db";
|
||||
import { requireAuth, requireAdmin } from "../middlewares/auth";
|
||||
import {
|
||||
CreateAppBody,
|
||||
@@ -13,13 +19,76 @@ import {
|
||||
|
||||
const router: IRouter = Router();
|
||||
|
||||
router.get("/apps", requireAuth, async (_req, res): Promise<void> => {
|
||||
router.get("/apps", requireAuth, async (req, res): Promise<void> => {
|
||||
const userId = req.session.userId!;
|
||||
|
||||
const userRoleRows = await db
|
||||
.select({ roleName: rolesTable.name, roleId: userRolesTable.roleId })
|
||||
.from(userRolesTable)
|
||||
.innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id))
|
||||
.where(eq(userRolesTable.userId, userId));
|
||||
|
||||
const isAdmin = userRoleRows.some((r) => r.roleName === "admin");
|
||||
|
||||
if (isAdmin) {
|
||||
const apps = await db
|
||||
.select()
|
||||
.from(appsTable)
|
||||
.where(eq(appsTable.isActive, true))
|
||||
.orderBy(asc(appsTable.sortOrder), asc(appsTable.nameEn));
|
||||
res.json(apps);
|
||||
return;
|
||||
}
|
||||
|
||||
const userRoleIds = userRoleRows.map((r) => r.roleId);
|
||||
|
||||
const userPermissionIds = userRoleIds.length > 0
|
||||
? (await db
|
||||
.select({ permissionId: rolePermissionsTable.permissionId })
|
||||
.from(rolePermissionsTable)
|
||||
.where(inArray(rolePermissionsTable.roleId, userRoleIds))
|
||||
).map((r) => r.permissionId)
|
||||
: [];
|
||||
|
||||
const apps = await db
|
||||
.select()
|
||||
.from(appsTable)
|
||||
.where(eq(appsTable.isActive, true))
|
||||
.where(
|
||||
eq(appsTable.isActive, true),
|
||||
)
|
||||
.orderBy(asc(appsTable.sortOrder), asc(appsTable.nameEn));
|
||||
res.json(apps);
|
||||
|
||||
const appsWithPermissions = await db
|
||||
.select({ appId: appPermissionsTable.appId })
|
||||
.from(appPermissionsTable);
|
||||
|
||||
const restrictedAppIds = new Set(appsWithPermissions.map((r) => r.appId));
|
||||
|
||||
if (restrictedAppIds.size === 0) {
|
||||
const unrestricted = await db
|
||||
.select()
|
||||
.from(appsTable)
|
||||
.where(eq(appsTable.isActive, true))
|
||||
.orderBy(asc(appsTable.sortOrder), asc(appsTable.nameEn));
|
||||
res.json(unrestricted);
|
||||
return;
|
||||
}
|
||||
|
||||
const allowedRestrictedAppIds = userPermissionIds.length > 0
|
||||
? (await db
|
||||
.select({ appId: appPermissionsTable.appId })
|
||||
.from(appPermissionsTable)
|
||||
.where(inArray(appPermissionsTable.permissionId, userPermissionIds))
|
||||
).map((r) => r.appId)
|
||||
: [];
|
||||
|
||||
const allowedAppIdSet = new Set(allowedRestrictedAppIds);
|
||||
|
||||
const visibleApps = apps.filter(
|
||||
(app) => !restrictedAppIds.has(app.id) || allowedAppIdSet.has(app.id),
|
||||
);
|
||||
|
||||
res.json(visibleApps);
|
||||
});
|
||||
|
||||
router.post("/apps", requireAdmin, async (req, res): Promise<void> => {
|
||||
|
||||
@@ -4,11 +4,13 @@ import { db } from "@workspace/db";
|
||||
import { usersTable } from "@workspace/db";
|
||||
import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth";
|
||||
import {
|
||||
RegisterBody,
|
||||
GetUserParams,
|
||||
UpdateUserParams,
|
||||
UpdateUserBody,
|
||||
DeleteUserParams,
|
||||
} from "@workspace/api-zod";
|
||||
import bcrypt from "bcryptjs";
|
||||
|
||||
const router: IRouter = Router();
|
||||
|
||||
@@ -42,6 +44,45 @@ function buildUserProfile(user: typeof usersTable.$inferSelect, roles: string[])
|
||||
};
|
||||
}
|
||||
|
||||
router.post("/users", requireAdmin, async (req, res): Promise<void> => {
|
||||
const parsed = RegisterBody.safeParse(req.body);
|
||||
if (!parsed.success) {
|
||||
res.status(400).json({ error: parsed.error.message });
|
||||
return;
|
||||
}
|
||||
|
||||
const existing = await db
|
||||
.select({ id: usersTable.id })
|
||||
.from(usersTable)
|
||||
.where(eq(usersTable.username, parsed.data.username));
|
||||
|
||||
if (existing.length > 0) {
|
||||
res.status(409).json({ error: "Username already taken" });
|
||||
return;
|
||||
}
|
||||
|
||||
const passwordHash = await bcrypt.hash(parsed.data.password, 12);
|
||||
|
||||
const [user] = await db
|
||||
.insert(usersTable)
|
||||
.values({
|
||||
username: parsed.data.username,
|
||||
email: parsed.data.email,
|
||||
passwordHash,
|
||||
displayNameAr: parsed.data.displayNameAr ?? null,
|
||||
displayNameEn: parsed.data.displayNameEn ?? null,
|
||||
preferredLanguage: parsed.data.preferredLanguage,
|
||||
})
|
||||
.returning();
|
||||
|
||||
if (!user) {
|
||||
res.status(500).json({ error: "Failed to create user" });
|
||||
return;
|
||||
}
|
||||
|
||||
res.status(201).json(buildUserProfile(user, []));
|
||||
});
|
||||
|
||||
router.get("/users", requireAdmin, async (_req, res): Promise<void> => {
|
||||
const users = await db.select().from(usersTable).orderBy(usersTable.createdAt);
|
||||
const usersWithRoles = await Promise.all(
|
||||
|
||||
@@ -4,23 +4,16 @@ import { cva, type VariantProps } from "class-variance-authority"
|
||||
import { cn } from "@/lib/utils"
|
||||
|
||||
const badgeVariants = cva(
|
||||
// @replit
|
||||
// Whitespace-nowrap: Badges should never wrap.
|
||||
"whitespace-nowrap inline-flex items-center rounded-md border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2" +
|
||||
" hover-elevate ",
|
||||
"whitespace-nowrap inline-flex items-center rounded-md border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 hover-elevate",
|
||||
{
|
||||
variants: {
|
||||
variant: {
|
||||
default:
|
||||
// @replit shadow-xs instead of shadow, no hover because we use hover-elevate
|
||||
"border-transparent bg-primary text-primary-foreground shadow-xs",
|
||||
secondary:
|
||||
// @replit no hover because we use hover-elevate
|
||||
"border-transparent bg-secondary text-secondary-foreground",
|
||||
destructive:
|
||||
// @replit shadow-xs instead of shadow, no hover because we use hover-elevate
|
||||
"border-transparent bg-destructive text-destructive-foreground shadow-xs",
|
||||
// @replit shadow-xs" - use badge outline variable
|
||||
outline: "text-foreground border [border-color:var(--badge-outline)]",
|
||||
},
|
||||
},
|
||||
|
||||
@@ -5,30 +5,22 @@ import { cva, type VariantProps } from "class-variance-authority"
|
||||
import { cn } from "@/lib/utils"
|
||||
|
||||
const buttonVariants = cva(
|
||||
"inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0" +
|
||||
" hover-elevate active-elevate-2",
|
||||
"inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0 hover-elevate active-elevate-2",
|
||||
{
|
||||
variants: {
|
||||
variant: {
|
||||
default:
|
||||
// @replit: no hover, and add primary border
|
||||
"bg-primary text-primary-foreground border border-primary-border",
|
||||
"bg-primary text-primary-foreground border border-primary-border",
|
||||
destructive:
|
||||
"bg-destructive text-destructive-foreground shadow-sm border-destructive-border",
|
||||
outline:
|
||||
// @replit Shows the background color of whatever card / sidebar / accent background it is inside of.
|
||||
// Inherits the current text color. Uses shadow-xs. no shadow on active
|
||||
// No hover state
|
||||
" border [border-color:var(--button-outline)] shadow-xs active:shadow-none ",
|
||||
"border [border-color:var(--button-outline)] shadow-xs active:shadow-none",
|
||||
secondary:
|
||||
// @replit border, no hover, no shadow, secondary border.
|
||||
"border bg-secondary text-secondary-foreground border border-secondary-border ",
|
||||
// @replit no hover, transparent border
|
||||
"border bg-secondary text-secondary-foreground border border-secondary-border",
|
||||
ghost: "border border-transparent",
|
||||
link: "text-primary underline-offset-4 hover:underline",
|
||||
},
|
||||
size: {
|
||||
// @replit changed sizes
|
||||
default: "min-h-9 px-4 py-2",
|
||||
sm: "min-h-8 rounded-md px-3 text-xs",
|
||||
lg: "min-h-10 rounded-md px-8",
|
||||
|
||||
@@ -89,6 +89,7 @@
|
||||
"loading": "جاري التحميل...",
|
||||
"error": "حدث خطأ",
|
||||
"success": "تمت العملية بنجاح",
|
||||
"language": "English"
|
||||
"language": "English",
|
||||
"appName": "نظام TeaBoy"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,6 +89,7 @@
|
||||
"loading": "Loading...",
|
||||
"error": "An error occurred",
|
||||
"success": "Success",
|
||||
"language": "العربية"
|
||||
"language": "العربية",
|
||||
"appName": "TeaBoy OS"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,7 +46,7 @@ export default function LoginPage() {
|
||||
<div className="min-h-screen os-bg flex items-center justify-center p-4">
|
||||
<div className="glass-panel rounded-3xl p-8 w-full max-w-sm space-y-6">
|
||||
<div className="text-center space-y-1">
|
||||
<div className="text-4xl font-bold text-primary mb-2">TeaBoy OS</div>
|
||||
<div className="text-4xl font-bold text-primary mb-2">{t("common.appName")}</div>
|
||||
<h1 className="text-xl font-semibold text-foreground">{t("auth.welcomeBack")}</h1>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -48,7 +48,7 @@ export default function RegisterPage() {
|
||||
<div className="min-h-screen os-bg flex items-center justify-center p-4">
|
||||
<div className="glass-panel rounded-3xl p-8 w-full max-w-sm space-y-6">
|
||||
<div className="text-center space-y-1">
|
||||
<div className="text-4xl font-bold text-primary mb-2">TeaBoy OS</div>
|
||||
<div className="text-4xl font-bold text-primary mb-2">{t("common.appName")}</div>
|
||||
<h1 className="text-xl font-semibold text-foreground">{t("auth.createAccount")}</h1>
|
||||
</div>
|
||||
|
||||
|
||||
Reference in New Issue
Block a user