diff --git a/CHANGELOG.md b/CHANGELOG.md index 92960d6..766c7e3 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/backend/app/api/v1/endpoints/mail_accounts.py b/backend/app/api/v1/endpoints/mail_accounts.py index f5f28b2..196b142 100644 --- a/backend/app/api/v1/endpoints/mail_accounts.py +++ b/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) diff --git a/backend/app/models/schemas.py b/backend/app/models/schemas.py index 6321eb0..4c99c59 100644 --- a/backend/app/models/schemas.py +++ b/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) diff --git a/backend/requirements.txt b/backend/requirements.txt index 0707b88..2a11d52 100644 --- a/backend/requirements.txt +++ b/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) diff --git a/docs/TODO.md b/docs/TODO.md index f765e6f..2aa806e 100644 --- a/docs/TODO.md +++ b/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) diff --git a/frontend/src/components/AddMailAccountModal.tsx b/frontend/src/components/AddMailAccountModal.tsx index 8b3c834..30059ef 100644 --- a/frontend/src/components/AddMailAccountModal.tsx +++ b/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(account ? 'form' : 'provider'); + const [wizardStep, setWizardStep] = useState(isEditMode ? 'form' : 'provider'); const [formData, setFormData] = useState({ 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) => { @@ -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

- {account ? 'Edit Mail Account' : 'Add Mail Account'} + {isEditMode ? 'Edit Mail Account' : 'Add Mail Account'}

- {wizardStep === 'provider' && !account ? ( + {wizardStep === 'provider' && !isEditMode ? ( setWizardStep('form')} /> ) : (
- {!account && ( + {!isEditMode && (
+
@@ -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'} />
@@ -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" /> @@ -446,10 +480,10 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index ea515cf..a8b5d6c 100644 --- a/frontend/src/lib/api.ts +++ b/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 + data: MailAccountUpdate ): Promise { const response = await api.put(`/mail-accounts/${id}`, data); return response.data;