Fix mailbox edit form: pre-populate fields, prevent credential overwrite, lock immutable settings

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> Agent-Logs-Url: https://github.com/christianlouis/pop_puller_to_gmail/sessions/20fa7a89-23e5-462c-8b4d-9c6cdb4d0501
2026-03-25 23:00:00 +00:00
parent 6bea660d16
commit 20170f2f2d
5 changed files with 93 additions and 39 deletions
@@ -18,6 +18,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. In edit mode the username is now pre-populated with the account's `email_address`, marked as `required` only when creating, and rendered as disabled together with the other immutable connection fields (protocol, host, port, Use SSL/TLS). The Auto-Detect button is also hidden in edit mode since server settings are locked after creation.
+- 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 only the fields the backend accepts for update (`name`, `forward_to`, `delivery_method`, `is_enabled`, `check_interval_minutes`, `max_emails_per_check`, `delete_after_forward`, and optionally `password`).
 - Gmail API `verify_access()` returning 403 for tokens that lacked a read-capable scope: added `gmail.readonly` to all scope lists.

 - Fixed three ESLint errors that caused CI to fail: removed unused `_setUser` store binding and unused `useAuthStore` import from `login/page.tsx`; replaced unused `_err` catch binding with a bare `catch {}` in `login/page.tsx`; removed a `useEffect` in `settings/page.tsx` that called `setProfileForm` synchronously (flagged by `react-hooks/set-state-in-effect`) — the effect was redundant because `useState` already initialises the form from the auth store's `user` object, which is the same value passed as `initialData` to `useQuery`.
@@ -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)
@@ -215,6 +215,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: pre-populate username from `email_address`; make connection fields (username, protocol, host, port, SSL) read-only; remove hard `required` on username; send only `MailAccountUpdate`-compatible payload on edit; skip empty-string password both in the frontend payload and as a backend guard
 - [x] `DashboardLayout` with responsive sidebar
 - [x] `AuthGuard` for protected routes
 - [x] Fix wizard grey screen (Tailwind v4 `bg-opacity` → `/75` syntax, modal restructure)
@@ -2,9 +2,9 @@

 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 { X, Loader2, CheckCircle, XCircle, Lock } from 'lucide-react';
 import { ProviderWizard } from './ProviderWizard';

 interface AddMailAccountModalProps {
@@ -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?.email_address || '',
    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,26 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
  const handleSubmit = async (e: React.FormEvent) => {
    e.preventDefault();
    try {
-      await createMutation.mutateAsync(formData);
+      if (isEditMode) {
+        // For updates only send the fields the backend allows changing.
+        // Never send an empty password – the backend treats a non-empty value
+        // as an intentional credential change.
+        const updateData: MailAccountUpdate = {
+          name: formData.name,
+          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 +183,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 +194,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,19 +236,28 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                        name="username"
                        value={formData.username}
                        onChange={handleChange}
-                        required
-                        className="flex-1 px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
+                        required={!isEditMode}
+                        disabled={isEditMode}
+                        className="flex-1 px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:bg-gray-50 disabled:text-gray-500 disabled:cursor-not-allowed"
                        placeholder="user@example.com"
                      />
-                      <button
-                        type="button"
-                        onClick={handleAutoDetect}
-                        disabled={autoDetecting}
-                        className="px-4 py-2 bg-gray-100 text-gray-700 rounded-md hover:bg-gray-200 disabled:opacity-50"
-                      >
-                        {autoDetecting ? 'Detecting...' : 'Auto-Detect'}
-                      </button>
+                      {!isEditMode && (
+                        <button
+                          type="button"
+                          onClick={handleAutoDetect}
+                          disabled={autoDetecting}
+                          className="px-4 py-2 bg-gray-100 text-gray-700 rounded-md hover:bg-gray-200 disabled:opacity-50"
+                        >
+                          {autoDetecting ? 'Detecting...' : 'Auto-Detect'}
+                        </button>
+                      )}
                    </div>
+                    {isEditMode && (
+                      <p className="mt-1 text-xs text-gray-500 flex items-center gap-1">
+                        <Lock className="h-3 w-3" />
+                        Username and server settings cannot be changed after creation.
+                      </p>
+                    )}
                  </div>

                  <div>
@@ -234,9 +269,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>

@@ -249,7 +284,8 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                        name="protocol"
                        value={formData.protocol}
                        onChange={handleChange}
-                        className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
+                        disabled={isEditMode}
+                        className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:bg-gray-50 disabled:text-gray-500 disabled:cursor-not-allowed"
                      >
                        <option value="pop3">POP3</option>
                        <option value="pop3_ssl">POP3 (SSL)</option>
@@ -267,8 +303,9 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                        name="host"
                        value={formData.host}
                        onChange={handleChange}
-                        required
-                        className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
+                        required={!isEditMode}
+                        disabled={isEditMode}
+                        className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:bg-gray-50 disabled:text-gray-500 disabled:cursor-not-allowed"
                        placeholder="pop.gmail.com"
                      />
                    </div>
@@ -282,8 +319,9 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                        name="port"
                        value={formData.port}
                        onChange={handleChange}
-                        required
-                        className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500"
+                        required={!isEditMode}
+                        disabled={isEditMode}
+                        className="w-full px-3 py-2 border border-gray-300 rounded-md focus:outline-none focus:ring-2 focus:ring-blue-500 disabled:bg-gray-50 disabled:text-gray-500 disabled:cursor-not-allowed"
                      />
                    </div>
                  </div>
@@ -296,9 +334,10 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
                        id="use_ssl"
                        checked={formData.use_ssl}
                        onChange={handleChange}
-                        className="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded"
+                        disabled={isEditMode}
+                        className="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded disabled:cursor-not-allowed"
                      />
-                      <label htmlFor="use_ssl" className="ml-2 block text-sm text-gray-700">
+                      <label htmlFor="use_ssl" className={`ml-2 block text-sm ${isEditMode ? 'text-gray-400' : 'text-gray-700'}`}>
                        Use SSL/TLS
                      </label>
                    </div>
@@ -446,10 +485,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>
@@ -95,6 +95,17 @@ export interface MailAccountCreate {
  delete_after_forward?: boolean;
 }

+export interface MailAccountUpdate {
+  name?: 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;
@@ -220,7 +231,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;