Merge pull request #107 from christianlouis/copilot/check-mailbox-credentials
Fix: Test Connection always reports success regardless of auth result
This commit is contained in:
@@ -17,6 +17,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## v0.2.2 (2026-03-28)
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
- **Test Connection always showed "success"**: frontend never checked the `success` field
|
||||
returned by the backend — any HTTP 200 response (including `{success: false}`) was
|
||||
displayed as "Connection successful!". Now the actual `success` value is checked and
|
||||
authentication failures are shown as errors with the server's message.
|
||||
- **Test Connection in edit mode required password re-entry**: added backend endpoint
|
||||
`POST /mail-accounts/{account_id}/test` that decrypts and uses the stored credentials
|
||||
so existing accounts can be tested without re-typing the password.
|
||||
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
- Resolve semantic-release CHANGELOG.md not updating properly
|
||||
|
||||
@@ -8,7 +8,7 @@ from sqlalchemy import select, desc, func
|
||||
|
||||
from app.core.database import get_db
|
||||
from app.core.deps import get_current_active_user
|
||||
from app.core.security import encrypt_credential
|
||||
from app.core.security import encrypt_credential, decrypt_credential
|
||||
from app.models.database_models import (
|
||||
User,
|
||||
MailAccount,
|
||||
@@ -269,6 +269,39 @@ async def test_mail_connection(
|
||||
return MailAccountTestResponse(success=success, message=message)
|
||||
|
||||
|
||||
@router.post("/{account_id}/test", response_model=MailAccountTestResponse)
|
||||
async def test_existing_mail_connection(
|
||||
account_id: int,
|
||||
current_user: User = Depends(get_current_active_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Test connection for an existing mail account using its stored credentials"""
|
||||
result = await db.execute(
|
||||
select(MailAccount).where(
|
||||
MailAccount.id == account_id, MailAccount.user_id == current_user.id
|
||||
)
|
||||
)
|
||||
account = result.scalar_one_or_none()
|
||||
|
||||
if not account:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND, detail="Mail account not found"
|
||||
)
|
||||
|
||||
try:
|
||||
password = decrypt_credential(account.encrypted_password)
|
||||
except Exception:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
|
||||
detail="Failed to decrypt stored credentials",
|
||||
)
|
||||
|
||||
processor = MailProcessor(account, password)
|
||||
success, message = await processor.test_connection()
|
||||
|
||||
return MailAccountTestResponse(success=success, message=message)
|
||||
|
||||
|
||||
@router.post("/auto-detect", response_model=MailAccountAutoDetectResponse)
|
||||
async def auto_detect_mail_settings(
|
||||
detect_request: MailAccountAutoDetectRequest,
|
||||
|
||||
+2
-1
@@ -17,7 +17,8 @@ Comprehensive task breakdown for repository improvements and production readines
|
||||
- [x] Added GitOps auto-deployment step in `ci.yml` to update preprod k8s manifest in `k8s-cluster-state` repo.
|
||||
- [x] Fixed GitOps `update-k8s-manifest` job: added PAT availability check to skip gracefully when `GH_PAT` secret is not configured, fixing 403 "Write access to repository not granted" pipeline failure.
|
||||
- [x] Fixed Celery `TypeError: can't subtract offset-naive and offset-aware datetimes` in `process_all_enabled_accounts` — all mail accounts were silently skipped on every scheduled run.
|
||||
- [x] Changed Celery beat schedule to run `process_all_enabled_accounts` every minute so accounts configured with `check_interval_minutes = 1` are polled as expected.
|
||||
- [x] Fixed **Test Connection** always reporting success regardless of authentication outcome.
|
||||
- [x] Added `POST /mail-accounts/{account_id}/test` endpoint to test existing accounts with stored credentials.
|
||||
|
||||
## 🔴 Critical - Security (In Progress)
|
||||
|
||||
|
||||
@@ -115,24 +115,37 @@ export function AddMailAccountModal({ account, onClose }: AddMailAccountModalPro
|
||||
};
|
||||
|
||||
const handleTestConnection = async () => {
|
||||
if (!formData.username || !formData.password || !formData.host) {
|
||||
alert('Please fill in username, password, and host');
|
||||
return;
|
||||
}
|
||||
|
||||
setTestStatus('testing');
|
||||
setTestMessage('');
|
||||
try {
|
||||
await mailAccountsApi.test({
|
||||
protocol: formData.protocol,
|
||||
host: formData.host,
|
||||
port: formData.port,
|
||||
username: formData.username,
|
||||
password: formData.password,
|
||||
use_ssl: formData.use_ssl,
|
||||
});
|
||||
setTestStatus('success');
|
||||
setTestMessage('Connection successful!');
|
||||
let result: { success: boolean; message: string };
|
||||
|
||||
if (isEditMode && !formData.password && account?.id) {
|
||||
// Edit mode with no new password entered — test using stored credentials
|
||||
result = await mailAccountsApi.testExisting(account.id);
|
||||
} else {
|
||||
if (!formData.username || !formData.password || !formData.host) {
|
||||
setTestStatus('error');
|
||||
setTestMessage('Please fill in username, password, and host');
|
||||
return;
|
||||
}
|
||||
result = await mailAccountsApi.test({
|
||||
protocol: formData.protocol,
|
||||
host: formData.host,
|
||||
port: formData.port,
|
||||
username: formData.username,
|
||||
password: formData.password,
|
||||
use_ssl: formData.use_ssl,
|
||||
});
|
||||
}
|
||||
|
||||
if (result.success) {
|
||||
setTestStatus('success');
|
||||
setTestMessage(result.message);
|
||||
} else {
|
||||
setTestStatus('error');
|
||||
setTestMessage(result.message || 'Connection failed');
|
||||
}
|
||||
} catch (error) {
|
||||
setTestStatus('error');
|
||||
const errorMessage = error instanceof Error && 'response' in error
|
||||
|
||||
@@ -338,6 +338,13 @@ export const mailAccountsApi = {
|
||||
return response.data;
|
||||
},
|
||||
|
||||
async testExisting(accountId: number): Promise<{ success: boolean; message: string }> {
|
||||
const response = await api.post<{ success: boolean; message: string }>(
|
||||
`/mail-accounts/${accountId}/test`
|
||||
);
|
||||
return response.data;
|
||||
},
|
||||
|
||||
async autoDetect(
|
||||
emailAddress: string
|
||||
): Promise<{ success: boolean; suggestions: AutoDetectSuggestion[] }> {
|
||||
|
||||
Reference in New Issue
Block a user