refactor: fix all P0/P1/P2 bugs and architecture issues
Bug fixes (from bugs/ directory): - Fix cross-module DB queries in 9 modules (homework, grades, parent, diagnostic, elective, proctoring, notifications, scheduling, classes) by routing through data-access functions - Fix shared/lib <-> auth circular dependency via new session.ts module - Fix divide-by-zero guard in grades data-access - Fix audit export data truncation (paginated fetch for full datasets) - Fix missing transactions in homework grading and elective lottery - Fix missing revalidatePath in course-plans actions - Fix frontend permission checks using requirePermission instead of requireAuth - Fix dashboard role routing using session.user.roles - Fix student auth pattern (migrate getDemoStudentUser to users module) - Fix ActionState return type handling in components Code quality fixes: - Remove 60+ as type assertions (replace with type guards) - Remove non-null assertions (use optional chaining or explicit checks) - Convert dynamic imports to static imports (grades, diagnostic) - Add React.cache() wrapping for read functions - Parallelize independent queries with Promise.all - Add explicit return types to 30+ arrow functions - Replace any with unknown + type guards - Fix import type for type-only imports - Add Zod validation schemas for classes and diagnostic modules - Extract duplicate code (normalizeRoleName, normalizeBcryptHash, logger IP extraction) - Add console.error to silent catch blocks - Fix permission naming consistency (exam:proctor_read -> exam:proctor:read) Architecture doc sync: - Update 004_architecture_impact_map.md and 005_architecture_data.json - Update management-modules-audit.md for P0-7 cross-module fix Moved deleted proctoring event route to deletes/ folder.
This commit is contained in:
@@ -1,33 +1,40 @@
|
||||
"use server"
|
||||
|
||||
import { revalidatePath } from "next/cache"
|
||||
import { eq } from "drizzle-orm"
|
||||
import { compare, hash } from "bcryptjs"
|
||||
import { z } from "zod"
|
||||
|
||||
import { db } from "@/shared/db"
|
||||
import { users, passwordSecurity } from "@/shared/db/schema"
|
||||
import type { ActionState } from "@/shared/types/action-state"
|
||||
import { requireAuth, PermissionDeniedError } from "@/shared/lib/auth-guard"
|
||||
import { requirePermission, PermissionDeniedError } from "@/shared/lib/auth-guard"
|
||||
import { Permissions } from "@/shared/types/permissions"
|
||||
import { validatePassword } from "@/shared/lib/password-policy"
|
||||
import { rateLimit, rateLimitKey, RATE_LIMIT_RULES } from "@/shared/lib/rate-limit"
|
||||
import { normalizeBcryptHash } from "@/shared/lib/bcrypt-utils"
|
||||
|
||||
const normalizeBcryptHash = (value: string) => {
|
||||
if (value.startsWith("$2")) return value
|
||||
if (value.startsWith("$")) return `$2b${value}`
|
||||
return `$2b$${value}`
|
||||
}
|
||||
import {
|
||||
getPasswordSecurityByUserId,
|
||||
getUserPasswordHash,
|
||||
updateUserPassword,
|
||||
upsertPasswordSecurityOnPasswordChange,
|
||||
} from "./data-access"
|
||||
|
||||
const ChangePasswordSchema = z.object({
|
||||
currentPassword: z.string().min(1, "Current password is required"),
|
||||
newPassword: z.string().min(1, "New password is required"),
|
||||
confirmPassword: z.string().min(1, "Password confirmation is required"),
|
||||
})
|
||||
|
||||
/**
|
||||
* Change the current user's password. Requires only authentication
|
||||
* (no specific permission) since every user can manage their own
|
||||
* credentials. Rate-limited to slow brute-force of the current password.
|
||||
* Change the current user's password. Requires self-service profile update
|
||||
* permission (every authenticated user has it). Rate-limited to slow
|
||||
* brute-force of the current password.
|
||||
*/
|
||||
export async function changePasswordAction(
|
||||
prevState: ActionState<null>,
|
||||
formData: FormData
|
||||
): Promise<ActionState<null>> {
|
||||
try {
|
||||
const ctx = await requireAuth()
|
||||
const ctx = await requirePermission(Permissions.USER_PROFILE_UPDATE)
|
||||
const userId = ctx.userId
|
||||
|
||||
const limitKey = rateLimitKey("pwd-change", userId)
|
||||
@@ -36,13 +43,19 @@ export async function changePasswordAction(
|
||||
return { success: false, message: "Too many attempts. Please try again later." }
|
||||
}
|
||||
|
||||
const currentPassword = String(formData.get("currentPassword") ?? "")
|
||||
const newPassword = String(formData.get("newPassword") ?? "")
|
||||
const confirmPassword = String(formData.get("confirmPassword") ?? "")
|
||||
|
||||
if (!currentPassword || !newPassword || !confirmPassword) {
|
||||
return { success: false, message: "All fields are required" }
|
||||
const parsed = ChangePasswordSchema.safeParse({
|
||||
currentPassword: formData.get("currentPassword"),
|
||||
newPassword: formData.get("newPassword"),
|
||||
confirmPassword: formData.get("confirmPassword"),
|
||||
})
|
||||
if (!parsed.success) {
|
||||
return {
|
||||
success: false,
|
||||
message: parsed.error.issues[0]?.message ?? "Invalid form data",
|
||||
}
|
||||
}
|
||||
|
||||
const { currentPassword, newPassword, confirmPassword } = parsed.data
|
||||
if (newPassword !== confirmPassword) {
|
||||
return { success: false, message: "New passwords do not match" }
|
||||
}
|
||||
@@ -55,16 +68,17 @@ export async function changePasswordAction(
|
||||
return { success: false, message: validation.errors[0] ?? "Password does not meet requirements" }
|
||||
}
|
||||
|
||||
const [user] = await db
|
||||
.select({ id: users.id, password: users.password })
|
||||
.from(users)
|
||||
.where(eq(users.id, userId))
|
||||
.limit(1)
|
||||
if (!user || !user.password) {
|
||||
// Parallelize user and passwordSecurity queries
|
||||
const [userRecord, existingSecurity] = await Promise.all([
|
||||
getUserPasswordHash(userId),
|
||||
getPasswordSecurityByUserId(userId),
|
||||
])
|
||||
|
||||
if (!userRecord || !userRecord.password) {
|
||||
return { success: false, message: "User not found or no password set" }
|
||||
}
|
||||
|
||||
const storedHash = normalizeBcryptHash(user.password)
|
||||
const storedHash = normalizeBcryptHash(userRecord.password)
|
||||
if (!storedHash.startsWith("$2")) {
|
||||
return { success: false, message: "Stored password is invalid" }
|
||||
}
|
||||
@@ -75,34 +89,8 @@ export async function changePasswordAction(
|
||||
|
||||
const newHash = await hash(newPassword, 10)
|
||||
const now = new Date()
|
||||
await db
|
||||
.update(users)
|
||||
.set({ password: newHash, updatedAt: now })
|
||||
.where(eq(users.id, userId))
|
||||
|
||||
const [existing] = await db
|
||||
.select({ id: passwordSecurity.id })
|
||||
.from(passwordSecurity)
|
||||
.where(eq(passwordSecurity.userId, userId))
|
||||
.limit(1)
|
||||
if (existing) {
|
||||
await db
|
||||
.update(passwordSecurity)
|
||||
.set({
|
||||
lastPasswordChange: now,
|
||||
passwordChangedAt: now,
|
||||
mustChangePassword: false,
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(eq(passwordSecurity.userId, userId))
|
||||
} else {
|
||||
await db.insert(passwordSecurity).values({
|
||||
userId,
|
||||
lastPasswordChange: now,
|
||||
passwordChangedAt: now,
|
||||
mustChangePassword: false,
|
||||
})
|
||||
}
|
||||
await updateUserPassword(userId, newHash, now)
|
||||
await upsertPasswordSecurityOnPasswordChange(userId, now, existingSecurity)
|
||||
|
||||
revalidatePath("/settings")
|
||||
return { success: true, message: "Password changed successfully", data: null }
|
||||
|
||||
@@ -3,15 +3,23 @@
|
||||
import { z } from "zod"
|
||||
import { revalidatePath } from "next/cache"
|
||||
import { createId } from "@paralleldrive/cuid2"
|
||||
import { count, desc, eq } from "drizzle-orm"
|
||||
|
||||
import { db } from "@/shared/db"
|
||||
import { aiProviders } from "@/shared/db/schema"
|
||||
import type { ActionState } from "@/shared/types/action-state"
|
||||
import { requirePermission, PermissionDeniedError } from "@/shared/lib/auth-guard"
|
||||
import { Permissions } from "@/shared/types/permissions"
|
||||
import { encryptAiApiKey, getAiErrorMessage, testAiProviderById, testAiProviderConfig } from "@/shared/lib/ai"
|
||||
|
||||
import {
|
||||
countDefaultAiProviders,
|
||||
createAiProvider,
|
||||
getAiProviderForUpdate,
|
||||
getAiProviderSummaries as fetchAiProviderSummaries,
|
||||
updateAiProvider,
|
||||
} from "./data-access"
|
||||
import type { AiProviderSummary } from "./types"
|
||||
|
||||
export type { AiProviderSummary } from "./types"
|
||||
|
||||
const ProviderSchema = z.enum(["zhipu", "openai", "gemini", "custom"])
|
||||
|
||||
const AiProviderFormSchema = z.object({
|
||||
@@ -35,22 +43,12 @@ const AiProviderTestSchema = AiProviderFormSchema.extend({
|
||||
}
|
||||
})
|
||||
|
||||
export type AiProviderSummary = {
|
||||
id: string
|
||||
provider: z.infer<typeof ProviderSchema>
|
||||
baseUrl: string | null
|
||||
model: string
|
||||
apiKeyLast4: string | null
|
||||
isDefault: boolean
|
||||
updatedAt: Date
|
||||
}
|
||||
|
||||
const ensureUser = async () => {
|
||||
const ensureUser = async (): Promise<{ id: string }> => {
|
||||
const ctx = await requirePermission(Permissions.AI_CONFIGURE)
|
||||
return { id: ctx.userId }
|
||||
}
|
||||
|
||||
const normalizeBaseUrl = (value: string | undefined) => {
|
||||
const normalizeBaseUrl = (value: string | undefined): string | null => {
|
||||
const raw = String(value ?? "").trim()
|
||||
if (!raw.length) return null
|
||||
const trimmed = raw.replace(/\/+$/, "")
|
||||
@@ -61,19 +59,7 @@ const normalizeBaseUrl = (value: string | undefined) => {
|
||||
|
||||
export async function getAiProviderSummaries(): Promise<AiProviderSummary[]> {
|
||||
await ensureUser()
|
||||
const rows = await db
|
||||
.select({
|
||||
id: aiProviders.id,
|
||||
provider: aiProviders.provider,
|
||||
baseUrl: aiProviders.baseUrl,
|
||||
model: aiProviders.model,
|
||||
apiKeyLast4: aiProviders.apiKeyLast4,
|
||||
isDefault: aiProviders.isDefault,
|
||||
updatedAt: aiProviders.updatedAt,
|
||||
})
|
||||
.from(aiProviders)
|
||||
.orderBy(desc(aiProviders.updatedAt))
|
||||
return rows
|
||||
return fetchAiProviderSummaries()
|
||||
}
|
||||
|
||||
export async function upsertAiProviderAction(
|
||||
@@ -92,51 +78,39 @@ export async function upsertAiProviderAction(
|
||||
return { success: false, message: "Base URL is required for this provider" }
|
||||
}
|
||||
|
||||
const [defaultRow] = await db
|
||||
.select({ value: count() })
|
||||
.from(aiProviders)
|
||||
.where(eq(aiProviders.isDefault, true))
|
||||
const defaultCount = Number(defaultRow?.value ?? 0)
|
||||
// Parallelize default-count and existing-provider queries
|
||||
const [defaultCount, existing] = await Promise.all([
|
||||
countDefaultAiProviders(),
|
||||
payload.id ? getAiProviderForUpdate(payload.id) : Promise.resolve(null),
|
||||
])
|
||||
const hasDefault = defaultCount > 0
|
||||
|
||||
if (payload.id) {
|
||||
const id = payload.id
|
||||
const [existing] = await db
|
||||
.select({
|
||||
id: aiProviders.id,
|
||||
apiKeyEncrypted: aiProviders.apiKeyEncrypted,
|
||||
apiKeyLast4: aiProviders.apiKeyLast4,
|
||||
isDefault: aiProviders.isDefault,
|
||||
})
|
||||
.from(aiProviders)
|
||||
.where(eq(aiProviders.id, id))
|
||||
.limit(1)
|
||||
if (!existing) return { success: false, message: "AI provider not found" }
|
||||
|
||||
const nextKey = payload.apiKey?.trim()
|
||||
const encrypted = nextKey ? encryptAiApiKey(nextKey) : existing.apiKeyEncrypted
|
||||
const last4 = nextKey ? nextKey.slice(-4) : existing.apiKeyLast4
|
||||
|
||||
const nextIsDefault =
|
||||
payload.isDefault === false && existing.isDefault && defaultCount <= 1 ? true : payload.isDefault ?? existing.isDefault
|
||||
const isNextDefault =
|
||||
payload.isDefault === false && existing.isDefault && defaultCount <= 1
|
||||
? true
|
||||
: payload.isDefault ?? existing.isDefault
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
if (payload.isDefault) {
|
||||
await tx.update(aiProviders).set({ isDefault: false }).where(eq(aiProviders.isDefault, true))
|
||||
}
|
||||
await tx
|
||||
.update(aiProviders)
|
||||
.set({
|
||||
provider: payload.provider,
|
||||
baseUrl,
|
||||
model: payload.model,
|
||||
apiKeyEncrypted: encrypted,
|
||||
apiKeyLast4: last4,
|
||||
isDefault: nextIsDefault,
|
||||
updatedBy: user.id,
|
||||
})
|
||||
.where(eq(aiProviders.id, id))
|
||||
})
|
||||
await updateAiProvider(
|
||||
id,
|
||||
{
|
||||
provider: payload.provider,
|
||||
baseUrl,
|
||||
model: payload.model,
|
||||
apiKeyEncrypted: encrypted,
|
||||
apiKeyLast4: last4,
|
||||
isDefault: isNextDefault,
|
||||
updatedBy: user.id,
|
||||
},
|
||||
payload.isDefault === true
|
||||
)
|
||||
|
||||
revalidatePath("/settings")
|
||||
return { success: true, message: "AI provider updated", data: id }
|
||||
@@ -149,24 +123,22 @@ export async function upsertAiProviderAction(
|
||||
const id = createId()
|
||||
const encrypted = encryptAiApiKey(payload.apiKey.trim())
|
||||
const last4 = payload.apiKey.trim().slice(-4)
|
||||
const makeDefault = payload.isDefault ?? !hasDefault
|
||||
const shouldMakeDefault = payload.isDefault ?? !hasDefault
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
if (makeDefault) {
|
||||
await tx.update(aiProviders).set({ isDefault: false }).where(eq(aiProviders.isDefault, true))
|
||||
}
|
||||
await tx.insert(aiProviders).values({
|
||||
await createAiProvider(
|
||||
{
|
||||
id,
|
||||
provider: payload.provider,
|
||||
baseUrl,
|
||||
model: payload.model,
|
||||
apiKeyEncrypted: encrypted,
|
||||
apiKeyLast4: last4,
|
||||
isDefault: makeDefault,
|
||||
isDefault: shouldMakeDefault,
|
||||
createdBy: user.id,
|
||||
updatedBy: user.id,
|
||||
})
|
||||
})
|
||||
},
|
||||
shouldMakeDefault
|
||||
)
|
||||
|
||||
revalidatePath("/settings")
|
||||
return { success: true, message: "AI provider created", data: id }
|
||||
|
||||
@@ -47,14 +47,18 @@ export function ProfileSettingsForm({ user }: { user: UserProfile }) {
|
||||
function onSubmit(data: ProfileFormValues) {
|
||||
startTransition(async () => {
|
||||
try {
|
||||
await updateUserProfile({
|
||||
const result = await updateUserProfile({
|
||||
name: data.name,
|
||||
phone: data.phone || undefined,
|
||||
address: data.address || undefined,
|
||||
gender: data.gender || undefined,
|
||||
age: data.age || undefined,
|
||||
})
|
||||
toast.success("Profile updated successfully")
|
||||
if (result.success) {
|
||||
toast.success("Profile updated successfully")
|
||||
} else {
|
||||
toast.error(result.message || "Failed to update profile")
|
||||
}
|
||||
} catch (error) {
|
||||
toast.error("Failed to update profile")
|
||||
console.error(error)
|
||||
|
||||
172
src/modules/settings/data-access.ts
Normal file
172
src/modules/settings/data-access.ts
Normal file
@@ -0,0 +1,172 @@
|
||||
import "server-only"
|
||||
|
||||
import { count, desc, eq } from "drizzle-orm"
|
||||
|
||||
import { db } from "@/shared/db"
|
||||
import { aiProviders, passwordSecurity, users } from "@/shared/db/schema"
|
||||
|
||||
import type { AiProviderExisting, AiProviderName, AiProviderSummary } from "./types"
|
||||
|
||||
// --- AI Provider operations ---
|
||||
|
||||
export async function getAiProviderSummaries(): Promise<AiProviderSummary[]> {
|
||||
const rows = await db
|
||||
.select({
|
||||
id: aiProviders.id,
|
||||
provider: aiProviders.provider,
|
||||
baseUrl: aiProviders.baseUrl,
|
||||
model: aiProviders.model,
|
||||
apiKeyLast4: aiProviders.apiKeyLast4,
|
||||
isDefault: aiProviders.isDefault,
|
||||
updatedAt: aiProviders.updatedAt,
|
||||
})
|
||||
.from(aiProviders)
|
||||
.orderBy(desc(aiProviders.updatedAt))
|
||||
return rows
|
||||
}
|
||||
|
||||
export async function countDefaultAiProviders(): Promise<number> {
|
||||
const [row] = await db
|
||||
.select({ value: count() })
|
||||
.from(aiProviders)
|
||||
.where(eq(aiProviders.isDefault, true))
|
||||
return Number(row?.value ?? 0)
|
||||
}
|
||||
|
||||
export async function getAiProviderForUpdate(id: string): Promise<AiProviderExisting | null> {
|
||||
const [row] = await db
|
||||
.select({
|
||||
id: aiProviders.id,
|
||||
apiKeyEncrypted: aiProviders.apiKeyEncrypted,
|
||||
apiKeyLast4: aiProviders.apiKeyLast4,
|
||||
isDefault: aiProviders.isDefault,
|
||||
})
|
||||
.from(aiProviders)
|
||||
.where(eq(aiProviders.id, id))
|
||||
.limit(1)
|
||||
return row ?? null
|
||||
}
|
||||
|
||||
export async function updateAiProvider(
|
||||
id: string,
|
||||
data: {
|
||||
provider: AiProviderName
|
||||
baseUrl: string | null
|
||||
model: string
|
||||
apiKeyEncrypted: string
|
||||
apiKeyLast4: string | null
|
||||
isDefault: boolean
|
||||
updatedBy: string
|
||||
},
|
||||
resetOtherDefaults: boolean
|
||||
): Promise<void> {
|
||||
await db.transaction(async (tx) => {
|
||||
if (resetOtherDefaults) {
|
||||
await tx.update(aiProviders).set({ isDefault: false }).where(eq(aiProviders.isDefault, true))
|
||||
}
|
||||
await tx
|
||||
.update(aiProviders)
|
||||
.set({
|
||||
provider: data.provider,
|
||||
baseUrl: data.baseUrl,
|
||||
model: data.model,
|
||||
apiKeyEncrypted: data.apiKeyEncrypted,
|
||||
apiKeyLast4: data.apiKeyLast4,
|
||||
isDefault: data.isDefault,
|
||||
updatedBy: data.updatedBy,
|
||||
})
|
||||
.where(eq(aiProviders.id, id))
|
||||
})
|
||||
}
|
||||
|
||||
export async function createAiProvider(
|
||||
data: {
|
||||
id: string
|
||||
provider: AiProviderName
|
||||
baseUrl: string | null
|
||||
model: string
|
||||
apiKeyEncrypted: string
|
||||
apiKeyLast4: string | null
|
||||
isDefault: boolean
|
||||
createdBy: string
|
||||
updatedBy: string
|
||||
},
|
||||
resetOtherDefaults: boolean
|
||||
): Promise<void> {
|
||||
await db.transaction(async (tx) => {
|
||||
if (resetOtherDefaults) {
|
||||
await tx.update(aiProviders).set({ isDefault: false }).where(eq(aiProviders.isDefault, true))
|
||||
}
|
||||
await tx.insert(aiProviders).values({
|
||||
id: data.id,
|
||||
provider: data.provider,
|
||||
baseUrl: data.baseUrl,
|
||||
model: data.model,
|
||||
apiKeyEncrypted: data.apiKeyEncrypted,
|
||||
apiKeyLast4: data.apiKeyLast4,
|
||||
isDefault: data.isDefault,
|
||||
createdBy: data.createdBy,
|
||||
updatedBy: data.updatedBy,
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// --- Password change operations ---
|
||||
|
||||
export async function getUserPasswordHash(
|
||||
userId: string
|
||||
): Promise<{ password: string | null } | null> {
|
||||
const [row] = await db
|
||||
.select({ password: users.password })
|
||||
.from(users)
|
||||
.where(eq(users.id, userId))
|
||||
.limit(1)
|
||||
return row ?? null
|
||||
}
|
||||
|
||||
export async function getPasswordSecurityByUserId(
|
||||
userId: string
|
||||
): Promise<{ id: string } | null> {
|
||||
const [row] = await db
|
||||
.select({ id: passwordSecurity.id })
|
||||
.from(passwordSecurity)
|
||||
.where(eq(passwordSecurity.userId, userId))
|
||||
.limit(1)
|
||||
return row ?? null
|
||||
}
|
||||
|
||||
export async function updateUserPassword(
|
||||
userId: string,
|
||||
newHash: string,
|
||||
now: Date
|
||||
): Promise<void> {
|
||||
await db
|
||||
.update(users)
|
||||
.set({ password: newHash, updatedAt: now })
|
||||
.where(eq(users.id, userId))
|
||||
}
|
||||
|
||||
export async function upsertPasswordSecurityOnPasswordChange(
|
||||
userId: string,
|
||||
now: Date,
|
||||
existing: { id: string } | null
|
||||
): Promise<void> {
|
||||
if (existing) {
|
||||
await db
|
||||
.update(passwordSecurity)
|
||||
.set({
|
||||
lastPasswordChange: now,
|
||||
passwordChangedAt: now,
|
||||
mustChangePassword: false,
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(eq(passwordSecurity.userId, userId))
|
||||
} else {
|
||||
await db.insert(passwordSecurity).values({
|
||||
userId,
|
||||
lastPasswordChange: now,
|
||||
passwordChangedAt: now,
|
||||
mustChangePassword: false,
|
||||
})
|
||||
}
|
||||
}
|
||||
18
src/modules/settings/types.ts
Normal file
18
src/modules/settings/types.ts
Normal file
@@ -0,0 +1,18 @@
|
||||
export type AiProviderName = "zhipu" | "openai" | "gemini" | "custom"
|
||||
|
||||
export interface AiProviderSummary {
|
||||
id: string
|
||||
provider: AiProviderName
|
||||
baseUrl: string | null
|
||||
model: string
|
||||
apiKeyLast4: string | null
|
||||
isDefault: boolean
|
||||
updatedAt: Date
|
||||
}
|
||||
|
||||
export interface AiProviderExisting {
|
||||
id: string
|
||||
apiKeyEncrypted: string
|
||||
apiKeyLast4: string | null
|
||||
isDefault: boolean
|
||||
}
|
||||
Reference in New Issue
Block a user