Merge pull request #80 from christianlouis/copilot/fix-mailbox-parameter-editing

Fix mailbox edit form: all fields editable, pre-populated, credentials preserved

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Security
+- Upgraded `python-jose` from 3.3.0 to 3.5.0 to fix CVE: algorithm confusion vulnerability with OpenSSH ECDSA keys (affected versions < 3.4.0).
 - Upgraded `python-jose[cryptography]` from `3.3.0` to `3.4.0` to fix an algorithm-confusion vulnerability with OpenSSH ECDSA keys (CVE affects all versions < 3.4.0).
 
 ### Added
@@ -28,6 +29,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `auth_service.py` `get_google_user_info` now returns `access_token`, `refresh_token`, `expires_in`, and `scope` alongside user info so the login endpoint can persist Gmail credentials in the same request.
 
 ### Fixed
+- Fixed mailbox edit form: username field was marked `required` but was never pre-populated (the backend intentionally excludes credentials from responses), making it impossible to save edits without re-entering the username. The backend now returns `username` in `MailAccountResponse` so the edit form can pre-populate it. All connection fields (protocol, host, port, use\_ssl, username) are now fully editable in edit mode. The Auto-Detect button is also shown in edit mode to re-detect server settings after a protocol change.
+- Fixed mailbox edit form silently overwriting stored credentials with an empty string: when the password field was left blank during an edit the frontend sent `password: ""`, which the backend encrypted and stored, locking the user out. The frontend now only includes `password` in the update payload when it is non-empty, and the backend additionally guards against empty-string passwords.
+- Fixed mailbox edit form sending all `MailAccountCreate` fields (including immutable ones like `username`, `host`, `port`) on update requests. The submit handler now builds a `MailAccountUpdate` payload containing all editable fields; `MailAccountUpdate` now covers every field in `MailAccountBase` (protocol, host, port, use\_ssl, use\_tls, username, email\_address, and the previously supported subset).
 - Fixed `Exception terminating connection` error logged by Celery workers after every task run. The error was caused by `asyncio.run()` closing the event loop while the asyncpg connection pool still held open idle connections. The fix calls `await engine.dispose()` inside the task's `_run()` coroutine (within the same event loop) so all pooled connections are closed cleanly before the loop is torn down.
 - Gmail API `verify_access()` returning 403 for tokens that lacked a read-capable scope: added `gmail.readonly` to all scope lists.
 

backend/app/api/v1/endpoints/mail_accounts.py

@@ -146,9 +146,9 @@ async def update_mail_account(
     update_data = account_update.model_dump(exclude_unset=True)
 
     if "password" in update_data:
-        update_data["encrypted_password"] = encrypt_credential(
-            update_data.pop("password")
-        )
+        password = update_data.pop("password")
+        if password:  # Only update when a non-empty password is provided
+            update_data["encrypted_password"] = encrypt_credential(password)
 
     for field, value in update_data.items():
         setattr(account, field, value)

backend/app/models/schemas.py

@@ -120,6 +120,13 @@ class MailAccountCreate(MailAccountBase):
 
 class MailAccountUpdate(BaseModel):
     name: Optional[str] = Field(None, max_length=255)
+    email_address: Optional[EmailStr] = None
+    protocol: Optional[MailProtocol] = None
+    host: Optional[str] = Field(None, max_length=255)
+    port: Optional[int] = Field(None, gt=0, lt=65536)
+    use_ssl: Optional[bool] = None
+    use_tls: Optional[bool] = None
+    username: Optional[str] = Field(None, max_length=255)
     password: Optional[str] = None
     forward_to: Optional[EmailStr] = None
     delivery_method: Optional[DeliveryMethod] = None
@@ -145,9 +152,8 @@ class MailAccountResponse(MailAccountBase):
     created_at: datetime
     updated_at: datetime
 
-    # Don't expose password or username in responses
+    # Don't expose password in responses; username is safe to return
     password: str = Field(exclude=True, default="")
-    username: str = Field(exclude=True, default="")
 
     model_config = ConfigDict(from_attributes=True)
 

backend/requirements.txt

@@ -11,7 +11,8 @@ psycopg2-binary==2.9.11
 asyncpg==0.31.0
 
 # Authentication
-python-jose[cryptography]==3.4.0
+
+python-jose[cryptography]==3.5.0  # Updated: Fixed algorithm confusion with OpenSSH ECDSA keys (was 3.3.0)
 bcrypt==4.3.0
 python-multipart==0.0.22  # Updated: Fixed multiple vulnerabilities (was 0.0.6)
 authlib==1.6.9  # Updated: Fixed OIDC hash binding, JWE RSA1_5 padding oracle, alg:none bypass, JWK header injection (was 1.6.6)

docs/TODO.md

@@ -11,6 +11,7 @@ Comprehensive task breakdown for repository improvements and production readines
 - [x] Implement CSRF protection middleware
 - [x] Document all error codes in docs/ERRORS.md
 - [x] Create security ADR (Architecture Decision Records)
+- [x] Upgrade `python-jose` 3.3.0 → 3.5.0 (algorithm confusion with OpenSSH ECDSA keys, CVE, affected < 3.4.0)
 
 ### In Progress 🔨
 - [ ] Enable rate limiting per user/tier
@@ -217,6 +218,7 @@ because the API client layer is missing.
 - [x] Mail accounts list with CRUD operations + enable/disable toggle
 - [x] Settings page — Profile, Gmail API connection, SMTP relay, Account info, Security
 - [x] `AddMailAccountModal` component (auto-detect, test connection, all required fields, is_enabled checkbox)
+- [x] Fix `AddMailAccountModal` edit mode: backend now returns `username` in `MailAccountResponse`; all fields (including protocol, host, port, use\_ssl, username) are editable in edit mode and pre-populated from the stored account; Auto-Detect is shown in edit mode too; only password is omitted from the update payload when left blank
 - [x] `DashboardLayout` with responsive sidebar
 - [x] `AuthGuard` for protected routes
 - [x] Fix wizard grey screen (Tailwind v4 `bg-opacity` → `/75` syntax, modal restructure)

frontend/src/components/AddMailAccountModal.tsx

@@ -2,7 +2,7 @@
 
 import { useState } from 'react';
 import { useMutation, useQueryClient } from '@tanstack/react-query';
-import { mailAccountsApi, MailAccount, MailAccountCreate } from '@/lib/api';
+import { mailAccountsApi, MailAccount, MailAccountCreate, MailAccountUpdate } from '@/lib/api';
 import { useAuthStore } from '@/store/authStore';
 import { X, Loader2, CheckCircle, XCircle } from 'lucide-react';
 import { ProviderWizard } from './ProviderWizard';
@@ -17,10 +17,11 @@ type WizardStep = 'provider' | 'form';
 export function AddMailAccountModal({ account, onClose }: AddMailAccountModalProps) {
   const queryClient = useQueryClient();
   const { user } = useAuthStore();
+  const isEditMode = !!account;
   const [testStatus, setTestStatus] = useState<'idle' | 'testing' | 'success' | 'error'>('idle');
   const [testMessage, setTestMessage] = useState('');
   const [autoDetecting, setAutoDetecting] = useState(false);
-  const [wizardStep, setWizardStep] = useState<WizardStep>(account ? 'form' : 'provider');
+  const [wizardStep, setWizardStep] = useState<WizardStep>(isEditMode ? 'form' : 'provider');
 
   const [formData, setFormData] = useState<MailAccountCreate>({
     name: account?.name || '',
@@ -28,7 +29,7 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
     protocol: account?.protocol || 'pop3_ssl',
     host: account?.host || '',
     port: account?.port || 995,
-    username: '',
+    username: account?.username || '',
     password: '',
     use_ssl: account?.use_ssl ?? true,
     use_tls: account?.use_tls ?? false,
@@ -40,13 +41,19 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
     delete_after_forward: account?.delete_after_forward ?? true,
   });
 
+  const onMutationSuccess = () => {
+    queryClient.invalidateQueries({ queryKey: ['mail-accounts'] });
+    onClose();
+  };
+
   const createMutation = useMutation({
-    mutationFn: (data: MailAccountCreate) =>
-      account ? mailAccountsApi.update(account.id, data) : mailAccountsApi.create(data),
-    onSuccess: () => {
-      queryClient.invalidateQueries({ queryKey: ['mail-accounts'] });
-      onClose();
-    },
+    mutationFn: (data: MailAccountCreate) => mailAccountsApi.create(data),
+    onSuccess: onMutationSuccess,
+  });
+
+  const updateMutation = useMutation({
+    mutationFn: (data: MailAccountUpdate) => mailAccountsApi.update(account!.id, data),
+    onSuccess: onMutationSuccess,
   });
 
   const handleChange = (e: React.ChangeEvent<HTMLInputElement | HTMLSelectElement>) => {
@@ -138,7 +145,33 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
   const handleSubmit = async (e: React.FormEvent) => {
     e.preventDefault();
     try {
-      await createMutation.mutateAsync(formData);
+      if (isEditMode) {
+        // Send all fields so the user can change any aspect of the account.
+        // Password is the only exception: omit it when blank so the stored
+        // credential is preserved.
+        const updateData: MailAccountUpdate = {
+          name: formData.name,
+          email_address: formData.email_address,
+          protocol: formData.protocol,
+          host: formData.host,
+          port: formData.port,
+          use_ssl: formData.use_ssl,
+          use_tls: formData.use_tls,
+          username: formData.username,
+          forward_to: formData.forward_to,
+          delivery_method: formData.delivery_method,
+          is_enabled: formData.is_enabled,
+          check_interval_minutes: formData.check_interval_minutes,
+          max_emails_per_check: formData.max_emails_per_check,
+          delete_after_forward: formData.delete_after_forward,
+        };
+        if (formData.password) {
+          updateData.password = formData.password;
+        }
+        await updateMutation.mutateAsync(updateData);
+      } else {
+        await createMutation.mutateAsync(formData);
+      }
     } catch (error) {
       const errorMessage = error instanceof Error && 'response' in error
         ? (error as { response?: { data?: { detail?: string } } }).response?.data?.detail
@@ -157,7 +190,7 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
             <div className="bg-white px-6 pt-6 pb-4">
               <div className="flex items-center justify-between mb-6">
                 <h3 className="text-lg font-semibold text-gray-900">
-                  {account ? 'Edit Mail Account' : 'Add Mail Account'}
+                  {isEditMode ? 'Edit Mail Account' : 'Add Mail Account'}
                 </h3>
                 <button
                   type="button"
@@ -168,14 +201,14 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                 </button>
               </div>
 
-              {wizardStep === 'provider' && !account ? (
+              {wizardStep === 'provider' && !isEditMode ? (
                 <ProviderWizard
                   onSelect={handleProviderSelect}
                   onManual={() => setWizardStep('form')}
                 />
               ) : (
                 <div className="space-y-4">
-                  {!account && (
+                  {!isEditMode && (
                     <button
                       type="button"
                       onClick={() => setWizardStep('provider')}
@@ -210,7 +243,7 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                         name="username"
                         value={formData.username}
                         onChange={handleChange}
-                        required
+                        required={!isEditMode}
                         className="flex-1 px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
                         placeholder="user@example.com"
                       />
@@ -223,6 +256,7 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                         {autoDetecting ? 'Detecting...' : 'Auto-Detect'}
                       </button>
                     </div>
+
                   </div>
 
                   <div>
@@ -234,9 +268,9 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                       name="password"
                       value={formData.password}
                       onChange={handleChange}
-                      required={!account}
+                      required={!isEditMode}
                       className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
-                      placeholder={account ? 'Leave blank to keep current password' : 'Password'}
+                      placeholder={isEditMode ? 'Leave blank to keep current password' : 'Password'}
                     />
                   </div>
 
@@ -267,7 +301,7 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                         name="host"
                         value={formData.host}
                         onChange={handleChange}
-                        required
+                        required={!isEditMode}
                         className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
                         placeholder="pop.gmail.com"
                       />
@@ -282,7 +316,7 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                         name="port"
                         value={formData.port}
                         onChange={handleChange}
-                        required
+                        required={!isEditMode}
                         className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
                       />
                     </div>
@@ -446,10 +480,10 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                   </button>
                   <button
                     type="submit"
-                    disabled={createMutation.isPending}
+                    disabled={createMutation.isPending || updateMutation.isPending}
                     className="px-4 py-2 bg-blue-600 text-white rounded-md hover:bg-blue-700 disabled:opacity-50"
                   >
-                    {createMutation.isPending ? 'Saving...' : 'Save'}
+                    {(createMutation.isPending || updateMutation.isPending) ? 'Saving...' : 'Save'}
                   </button>
                 </div>
               </div>

frontend/src/lib/api.ts

@@ -58,6 +58,7 @@ export interface MailAccount {
   port: number;
   use_ssl: boolean;
   use_tls: boolean;
+  username: string;
   forward_to: string;
   delivery_method: string;
   is_enabled: boolean;
@@ -95,6 +96,24 @@ export interface MailAccountCreate {
   delete_after_forward?: boolean;
 }
 
+export interface MailAccountUpdate {
+  name?: string;
+  email_address?: string;
+  protocol?: string;
+  host?: string;
+  port?: number;
+  use_ssl?: boolean;
+  use_tls?: boolean;
+  username?: string;
+  password?: string;
+  forward_to?: string;
+  delivery_method?: string;
+  is_enabled?: boolean;
+  check_interval_minutes?: number;
+  max_emails_per_check?: number;
+  delete_after_forward?: boolean;
+}
+
 export interface ProcessingRun {
   id: number;
   mail_account_id: number;
@@ -227,7 +246,7 @@ export const mailAccountsApi = {
 
   async update(
     id: number,
-    data: Partial<MailAccountCreate>
+    data: MailAccountUpdate
   ): Promise<MailAccount> {
     const response = await api.put<MailAccount>(`/mail-accounts/${id}`, data);
     return response.data;