diff --git a/frontend/src/components/admin/UserFormModal.tsx b/frontend/src/components/admin/UserFormModal.tsx index 119b443d3..ee273eb65 100644 --- a/frontend/src/components/admin/UserFormModal.tsx +++ b/frontend/src/components/admin/UserFormModal.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useCallback } from 'react'; import { motion, AnimatePresence } from 'framer-motion'; import { FiX, @@ -34,6 +34,7 @@ const UserFormModal: React.FC = ({ submitLabel, showPasswordFields = true, passwordOptional = false, + existingUsernames = [], isDark, themeStyles, }) => { @@ -42,21 +43,63 @@ const UserFormModal: React.FC = ({ const [showConfirmPassword, setShowConfirmPassword] = useState(false); const [formSubmitted, setFormSubmitted] = useState(false); const [usernameError, setUsernameError] = useState(null); + const [passwordError, setPasswordError] = useState(null); + const [passwordTouched, setPasswordTouched] = useState(false); - const validateUsername = (value: string) => { - if (!value.trim()) { - return null; // Don't show error for empty field until form submission - } - if (!/^[a-zA-Z0-9_-]+$/.test(value.trim())) { - return 'Username can only contain letters, numbers, underscore, and hyphen'; - } - return null; - }; + const validateUsername = useCallback( + (value: string) => { + const trimmed = value.trim(); + + if (!trimmed) { + return null; + } + + if (!/^[a-zA-Z0-9_-]+$/.test(trimmed)) { + return 'Username can only contain letters, numbers, underscore, and hyphen'; + } + + const normalized = trimmed.toLowerCase(); + const isTaken = existingUsernames.some(existing => existing.toLowerCase() === normalized); + + if (isTaken) { + return 'Username is already taken'; + } + + return null; + }, + [existingUsernames] + ); + + const validatePassword = useCallback( + (value: string) => { + const trimmed = value.trim(); + + if (!trimmed) { + return passwordOptional ? null : 'Password is required'; + } + + if (trimmed.length < 5) { + return 'Password must be at least 5 characters long'; + } + + return null; + }, + [passwordOptional] + ); useEffect(() => { const error = validateUsername(username); setUsernameError(error); - }, [username]); + }, [username, validateUsername]); + + useEffect(() => { + if (showPasswordFields && passwordTouched) { + const error = validatePassword(password); + setPasswordError(error); + } else { + setPasswordError(null); + } + }, [password, showPasswordFields, passwordTouched, validatePassword]); useEffect(() => { if (isAdmin) { @@ -64,12 +107,18 @@ const UserFormModal: React.FC = ({ permissionComponents.forEach(component => { setPermissionChange(component.id, 'write'); }); + } else { + // Reset permissions to their unselected state + permissionComponents.forEach(component => { + setPermissionChange(component.id, null); + }); } - }, [isAdmin]); + }, [isAdmin, permissionComponents, setPermissionChange]); useEffect(() => { if (!isOpen) { setFormSubmitted(false); + setPasswordTouched(false); } }, [isOpen]); @@ -92,31 +141,50 @@ const UserFormModal: React.FC = ({ }, [isOpen]); const validateForm = () => { - if (!username.trim()) { + const trimmedUsername = username.trim(); + const normalized = trimmedUsername.toLowerCase(); + + if (!trimmedUsername) { return { isValid: false, error: 'Username is required' }; } - if (!/^[a-zA-Z0-9_-]+$/.test(username.trim())) { + // Check format first + if (!/^[a-zA-Z0-9_-]+$/.test(trimmedUsername)) { return { isValid: false, error: 'Username can only contain letters, numbers, underscore, and hyphen', }; } + // Check for duplicate username + const isTaken = existingUsernames.some(existing => existing.toLowerCase() === normalized); + + if (isTaken) { + return { + isValid: false, + error: 'Username is already taken', + }; + } if (showPasswordFields) { // For Add User mode: password is required if (!passwordOptional) { - if (!password) { + const trimmedPassword = password.trim(); + const trimmedConfirm = confirmPassword.trim(); + + if (!trimmedPassword) { return { isValid: false, error: 'Password is required' }; } - if (password !== confirmPassword) { + if (trimmedPassword !== trimmedConfirm) { return { isValid: false, error: 'Passwords do not match' }; } } // For Edit User mode: password is optional, but if provided, must match else { // If user provides password in edit mode, both fields must match - if (password || confirmPassword) { - if (password !== confirmPassword) { + const trimmedPassword = password.trim(); + const trimmedConfirm = confirmPassword.trim(); + + if (trimmedPassword || trimmedConfirm) { + if (trimmedPassword !== trimmedConfirm) { return { isValid: false, error: 'Passwords do not match' }; } } @@ -208,22 +276,6 @@ const UserFormModal: React.FC = ({
- {formError && ( - - - {formError} - - )} -
{/* Username field */}
@@ -288,7 +340,7 @@ const UserFormModal: React.FC = ({ className="mt-1 flex items-center gap-1 text-xs text-red-500" > - {t('admin.users.errors.invalidUsername')} + {usernameError} )} {username && !usernameError && ( @@ -325,13 +377,29 @@ const UserFormModal: React.FC = ({ type={showPassword ? 'text' : 'password'} id="password" value={password} - onChange={e => setPassword(e.target.value)} - className="w-full rounded-lg border px-4 py-2.5 transition-all duration-200 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-opacity-50" + onChange={e => { + if (!passwordTouched) { + setPasswordTouched(true); + } + // Trim spaces so leading/trailing spaces are not stored or sent + setPassword(e.target.value.trim()); + }} + className={`w-full rounded-lg border px-4 py-2.5 transition-all duration-200 focus:outline-none focus:ring-2 focus:ring-opacity-50 ${ + passwordError + ? 'border-red-500 focus:ring-red-500' + : password && !passwordError && password.length >= 5 + ? 'border-green-500 focus:ring-green-500' + : 'focus:ring-blue-500' + }`} style={{ background: isDark ? 'rgba(31, 41, 55, 0.5)' : 'rgba(255, 255, 255, 0.8)', - borderColor: isDark - ? 'rgba(75, 85, 99, 0.3)' - : 'rgba(226, 232, 240, 0.8)', + borderColor: passwordError + ? '#ef4444' + : password && !passwordError && password.length >= 5 + ? '#10b981' + : isDark + ? 'rgba(75, 85, 99, 0.3)' + : 'rgba(226, 232, 240, 0.8)', color: themeStyles.colors.text.primary, boxShadow: isDark ? 'none' : 'inset 0 1px 2px rgba(0, 0, 0, 0.05)', }} @@ -358,6 +426,26 @@ const UserFormModal: React.FC = ({ )}
+ {password && passwordError && ( + + + {passwordError} + + )} + {password && !passwordError && password.length >= 5 && ( + + + Password length is valid + + )}
{/* Confirm Password field */} @@ -375,7 +463,7 @@ const UserFormModal: React.FC = ({ type={showConfirmPassword ? 'text' : 'password'} id="confirmPassword" value={confirmPassword} - onChange={e => setConfirmPassword(e.target.value)} + onChange={e => setConfirmPassword(e.target.value.trim())} className="w-full rounded-lg border px-4 py-2.5 transition-all duration-200 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-opacity-50" style={{ background: isDark ? 'rgba(31, 41, 55, 0.5)' : 'rgba(255, 255, 255, 0.8)', @@ -404,26 +492,32 @@ const UserFormModal: React.FC = ({ )} - {password && confirmPassword && password !== confirmPassword && ( - - - {t('admin.users.errors.passwordMismatch')} - - )} - {password && confirmPassword && password === confirmPassword && ( - - - Passwords match - - )} + {password && + confirmPassword && + password !== confirmPassword && + !passwordError && ( + + + {t('admin.users.errors.passwordMismatch')} + + )} + {password && + confirmPassword && + !passwordError && + password === confirmPassword && ( + + + Passwords match + + )} )} @@ -573,12 +667,7 @@ const UserFormModal: React.FC = ({ )} -
+
{ + if (axios.isAxiosError(error)) { + const data = error.response?.data as { + details?: string; + error?: string; + message?: string; + }; + + return data?.details || data?.error || data?.message || error.message || fallbackMessage; + } + + if (error instanceof Error && error.message) { + return error.message; + } + + return fallbackMessage; +}; + +const sanitizePermissions = (permissions: PermissionsMap): Record => { + return Object.entries(permissions).reduce( + (acc, [component, value]) => { + if (value) { + acc[component] = value; + } + return acc; + }, + {} as Record + ); +}; + const UserManagement = () => { const { t } = useTranslation(); const { theme } = useTheme(); @@ -177,21 +215,28 @@ const UserManagement = () => { const [password, setPassword] = useState(''); const [confirmPassword, setConfirmPassword] = useState(''); const [isUserAdmin, setIsUserAdmin] = useState(false); - const [userPermissions, setUserPermissions] = useState>({}); + const [userPermissions, setUserPermissions] = useState({}); + const [formError, setFormError] = useState(null); // Permissions components that can be managed - const permissionComponents: PermissionComponent[] = [ - { id: 'users', name: t('admin.users.permissions.users') }, - { id: 'resources', name: t('admin.users.permissions.resources') }, - { id: 'system', name: t('admin.users.permissions.system') }, - { id: 'dashboard', name: t('admin.users.permissions.dashboard') }, - ]; + const permissionComponents: PermissionComponent[] = useMemo( + () => [ + { id: 'users', name: t('admin.users.permissions.users') }, + { id: 'resources', name: t('admin.users.permissions.resources') }, + { id: 'system', name: t('admin.users.permissions.system') }, + { id: 'dashboard', name: t('admin.users.permissions.dashboard') }, + ], + [t] + ); // Available permission levels - const permissionLevels: PermissionLevel[] = [ - { id: 'read', name: t('admin.users.permissions.levels.read') }, - { id: 'write', name: t('admin.users.permissions.levels.write') }, - ]; + const permissionLevels: PermissionLevel[] = useMemo( + () => [ + { id: 'read', name: t('admin.users.permissions.levels.read') }, + { id: 'write', name: t('admin.users.permissions.levels.write') }, + ], + [t] + ); // Function to fetch users wrapped in useCallback const fetchUsers = useCallback(async () => { @@ -239,7 +284,7 @@ const UserManagement = () => { Object.keys(user.permissions || {}).some( key => key.toLowerCase().includes(lowerSearchTerm) || - user.permissions[key].toLowerCase().includes(lowerSearchTerm) + (user.permissions[key]?.toLowerCase().includes(lowerSearchTerm) ?? false) ) ); } @@ -314,6 +359,8 @@ const UserManagement = () => { }; const handleAddUser = async () => { + setFormError(null); + if (!username || (!passwordOptional && !password)) { toast.error(t('admin.users.errors.missingFields')); return; @@ -325,8 +372,8 @@ const UserManagement = () => { } try { - // If user is admin, set all permissions to write - const finalPermissions = { ...userPermissions }; + const finalPermissions = sanitizePermissions(userPermissions); + if (isUserAdmin) { permissionComponents.forEach(component => { finalPermissions[component.id] = 'write'; @@ -341,12 +388,15 @@ const UserManagement = () => { toast.success(t('admin.users.success.userAdded')); fetchUsers(); } catch (error) { + const errorMessage = extractApiErrorMessage(error, t('admin.users.errors.addFailed')); console.error('Error adding user:', error); - toast.error(t('admin.users.errors.addFailed')); + setFormError(errorMessage); } }; const handleEditUser = async () => { + setFormError(null); + if (!currentUser || !username) { toast.error(t('admin.users.errors.missingFields')); return; @@ -358,8 +408,8 @@ const UserManagement = () => { } try { - // If user is admin, set all permissions to write - const finalPermissions = { ...userPermissions }; + const finalPermissions = sanitizePermissions(userPermissions); + if (isUserAdmin) { permissionComponents.forEach(component => { finalPermissions[component.id] = 'write'; @@ -382,8 +432,9 @@ const UserManagement = () => { toast.success(t('admin.users.success.userUpdated')); fetchUsers(); } catch (error) { + const errorMessage = extractApiErrorMessage(error, t('admin.users.errors.updateFailed')); console.error('Error updating user:', error); - toast.error(t('admin.users.errors.updateFailed')); + setFormError(errorMessage); } }; @@ -426,6 +477,7 @@ const UserManagement = () => { setUserPermissions(user.permissions || {}); setPassword(''); setConfirmPassword(''); + setFormError(null); setShowEditModal(true); }; @@ -441,6 +493,7 @@ const UserManagement = () => { setIsUserAdmin(false); setUserPermissions({}); setCurrentUser(null); + setFormError(null); }; const closeModals = () => { @@ -450,12 +503,24 @@ const UserManagement = () => { resetForm(); }; - const handlePermissionChange = (component: string, permission: string) => { - setUserPermissions(prev => ({ - ...prev, - [component]: permission, - })); - }; + const handlePermissionChange = useCallback((component: string, permission: PermissionValue) => { + setUserPermissions(prev => { + if (prev[component] === permission) { + return prev; + } + + if (!permission) { + const rest = { ...prev }; + delete rest[component]; + return rest; + } + + return { + ...prev, + [component]: permission, + }; + }); + }, []); // Filter handling functions const toggleFilters = () => { @@ -996,6 +1061,7 @@ const UserManagement = () => { isOpen={showAddModal} onClose={closeModals} onSubmit={handleAddUser} + formError={formError ?? undefined} username={username} setUsername={setUsername} password={password} @@ -1009,6 +1075,7 @@ const UserManagement = () => { permissionComponents={permissionComponents} permissionLevels={permissionLevels} submitLabel={t('admin.users.actions.add')} + existingUsernames={users.map(u => u.username)} isDark={isDark} themeStyles={themeStyles} /> @@ -1019,6 +1086,7 @@ const UserManagement = () => { isOpen={showEditModal} onClose={closeModals} onSubmit={handleEditUser} + formError={formError ?? undefined} username={username} setUsername={setUsername} password={password} @@ -1034,6 +1102,7 @@ const UserManagement = () => { submitLabel={t('admin.users.actions.update')} showPasswordFields={true} passwordOptional={passwordOptional} + existingUsernames={users.map(u => u.username).filter(u => u !== currentUser?.username)} isDark={isDark} themeStyles={themeStyles} /> diff --git a/frontend/src/components/admin/UserTypes.ts b/frontend/src/components/admin/UserTypes.ts index 5179aa79b..5c88ca070 100644 --- a/frontend/src/components/admin/UserTypes.ts +++ b/frontend/src/components/admin/UserTypes.ts @@ -1,9 +1,12 @@ +export type PermissionValue = 'read' | 'write' | null; +export type PermissionsMap = Record; + // User data type export interface User { id?: number; username: string; is_admin: boolean; - permissions: Record; + permissions: PermissionsMap; created_at?: string; updated_at?: string; } @@ -25,7 +28,7 @@ export interface PermissionComponent { // Permission level type export interface PermissionLevel { - id: string; + id: Exclude; name: string; } @@ -59,13 +62,14 @@ export interface UserFormModalProps { setConfirmPassword: (confirmPassword: string) => void; isAdmin: boolean; setIsAdmin: (isAdmin: boolean) => void; - permissions: Record; - setPermissionChange: (component: string, permission: string) => void; + permissions: PermissionsMap; + setPermissionChange: (component: string, permission: PermissionValue) => void; permissionComponents: PermissionComponent[]; permissionLevels: PermissionLevel[]; submitLabel: string; showPasswordFields?: boolean; passwordOptional?: boolean; + existingUsernames?: string[]; // For checking username availability (only needed for add mode) isDark: boolean; themeStyles: ThemeStyles; } diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 48ba70362..323e8f022 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -50,8 +50,13 @@ api.interceptors.response.use( } const originalRequest = error.config as AxiosRequestConfig & { _retry?: boolean }; + const responseData = error.response?.data as { + details?: string; + message?: string; + error?: string; + }; const errorMessage = - error.response?.data?.message || error.response?.data?.error || error.message; + responseData?.details || responseData?.message || responseData?.error || error.message; const isAuthCheck = error.config?.url?.includes('/api/me'); const isLoginEndpoint = error.config?.url?.includes('/login');