Merge pull request #69 from christianlouis/copilot/debug-not-found-error
Fix /reports/{report_id} returning 404 Not Found
This commit is contained in:
Christian Krakau-Louis
@@ -1,5 +1,5 @@
|
||||
import logging
|
||||
from typing import List
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
from fastapi import APIRouter, File, HTTPException, UploadFile, status
|
||||
from pydantic import BaseModel
|
||||
@@ -374,3 +374,112 @@ async def delete_report(domain: str, report_id: str):
|
||||
success=True,
|
||||
message=f"Report '{report_id}' for domain '{domain}' deleted successfully.",
|
||||
)
|
||||
|
||||
|
||||
class ReportRecordDetail(BaseModel):
|
||||
"""Detailed record from a DMARC report"""
|
||||
|
||||
source_ip: str
|
||||
count: int
|
||||
disposition: str
|
||||
dkim_result: str
|
||||
spf_result: str
|
||||
header_from: str
|
||||
spf: Optional[List[Dict[str, Any]]] = None
|
||||
dkim: Optional[List[Dict[str, Any]]] = None
|
||||
|
||||
|
||||
class ReportPolicyDetail(BaseModel):
|
||||
"""Published policy from a DMARC report"""
|
||||
|
||||
p: str
|
||||
sp: str = ""
|
||||
pct: str = "100"
|
||||
|
||||
|
||||
class ReportSummaryDetail(BaseModel):
|
||||
"""Summary statistics for a DMARC report"""
|
||||
|
||||
total_count: int
|
||||
passed_count: int
|
||||
failed_count: int
|
||||
pass_rate: float
|
||||
|
||||
|
||||
class ReportDetail(BaseModel):
|
||||
"""Full detail of a single DMARC report"""
|
||||
|
||||
report_id: str
|
||||
org_name: str
|
||||
email: str
|
||||
domain: str
|
||||
begin_date: str
|
||||
end_date: str
|
||||
begin_timestamp: int
|
||||
end_timestamp: int
|
||||
policy: ReportPolicyDetail
|
||||
records: List[ReportRecordDetail]
|
||||
summary: ReportSummaryDetail
|
||||
|
||||
|
||||
@router.get("/{report_id}", response_model=ReportDetail)
|
||||
async def get_report_by_id(report_id: str):
|
||||
"""
|
||||
Get full details for a single DMARC report by its report ID.
|
||||
"""
|
||||
store = ReportStore.get_instance()
|
||||
report = store.get_report_by_id(report_id)
|
||||
|
||||
if report is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail=f"Report '{report_id}' not found.",
|
||||
)
|
||||
|
||||
# Normalize the policy field
|
||||
policy_val = report.get("policy", {})
|
||||
if isinstance(policy_val, str):
|
||||
policy_val = {"p": policy_val, "sp": "", "pct": "100"}
|
||||
policy_detail = ReportPolicyDetail(
|
||||
p=policy_val.get("p", "none"),
|
||||
sp=policy_val.get("sp", ""),
|
||||
pct=str(policy_val.get("pct", "100")),
|
||||
)
|
||||
|
||||
# Normalize records
|
||||
record_details = []
|
||||
for rec in report.get("records", []):
|
||||
record_details.append(
|
||||
ReportRecordDetail(
|
||||
source_ip=rec.get("source_ip", ""),
|
||||
count=rec.get("count", 0),
|
||||
disposition=rec.get("disposition", "none"),
|
||||
dkim_result=rec.get("dkim_result", ""),
|
||||
spf_result=rec.get("spf_result", ""),
|
||||
header_from=rec.get("header_from", ""),
|
||||
spf=rec.get("spf") if isinstance(rec.get("spf"), list) else None,
|
||||
dkim=rec.get("dkim") if isinstance(rec.get("dkim"), list) else None,
|
||||
)
|
||||
)
|
||||
|
||||
raw_summary = report.get("summary", {})
|
||||
summary_detail = ReportSummaryDetail(
|
||||
total_count=raw_summary.get("total_count", 0),
|
||||
passed_count=raw_summary.get("passed_count", 0),
|
||||
failed_count=raw_summary.get("failed_count", 0),
|
||||
pass_rate=raw_summary.get("pass_rate", 0.0),
|
||||
)
|
||||
|
||||
return ReportDetail(
|
||||
report_id=report.get("report_id", ""),
|
||||
org_name=report.get("org_name", ""),
|
||||
email=report.get("email", ""),
|
||||
domain=report.get("domain", ""),
|
||||
begin_date=str(report.get("begin_date", "")),
|
||||
end_date=str(report.get("end_date", "")),
|
||||
begin_timestamp=report.get("begin_timestamp", 0),
|
||||
end_timestamp=report.get("end_timestamp", 0),
|
||||
policy=policy_detail,
|
||||
records=record_details,
|
||||
summary=summary_detail,
|
||||
)
|
||||
|
||||
@@ -408,6 +408,14 @@ async def reports(request: Request):
|
||||
return templates.TemplateResponse(request, "reports.html")
|
||||
|
||||
|
||||
@app.get("/reports/{report_id}", response_class=HTMLResponse)
|
||||
async def report_detail(request: Request, report_id: str):
|
||||
"""View detailed information for a specific DMARC report"""
|
||||
return templates.TemplateResponse(
|
||||
request, "report_detail.html", {"report_id": report_id}
|
||||
)
|
||||
|
||||
|
||||
@app.get("/settings", response_class=HTMLResponse)
|
||||
async def settings_page(request: Request):
|
||||
return templates.TemplateResponse(request, "settings.html")
|
||||
|
||||
@@ -147,6 +147,22 @@ class ReportStore:
|
||||
"""
|
||||
return self.domain_summary
|
||||
|
||||
def get_report_by_id(self, report_id: str) -> Optional[Dict[str, Any]]:
|
||||
"""
|
||||
Find a report by its report_id across all domains.
|
||||
|
||||
Args:
|
||||
report_id: Report identifier from the DMARC report metadata
|
||||
|
||||
Returns:
|
||||
The report dictionary if found, None otherwise
|
||||
"""
|
||||
for reports in self.domain_reports.values():
|
||||
for report in reports:
|
||||
if report.get("report_id") == report_id:
|
||||
return report
|
||||
return None
|
||||
|
||||
def get_domain_reports(self, domain: str, limit: Optional[int] = None) -> List[Dict[str, Any]]:
|
||||
"""
|
||||
Get all reports for a domain
|
||||
|
||||
@@ -0,0 +1,276 @@
|
||||
{% extends "layouts/base.html" %}
|
||||
{% from "components/ui/card.html" import card, card_header, card_title, card_description, card_content %}
|
||||
{% from "components/ui/button.html" import button, button_link %}
|
||||
{% from "components/ui/table.html" import table, thead, tbody, tr, th, td %}
|
||||
|
||||
{% block title %}DMARQ - Report Detail{% endblock %}
|
||||
|
||||
{% block content %}
|
||||
<div class="container mx-auto py-4" x-data="reportDetailApp('{{ report_id }}')">
|
||||
<nav class="mb-4 text-sm">
|
||||
<ol class="flex items-center space-x-2">
|
||||
<li><a href="/" class="hover:text-primary">Dashboard</a></li>
|
||||
<li><span class="text-muted-foreground px-2">/</span></li>
|
||||
<li><a href="/reports" class="hover:text-primary">Reports</a></li>
|
||||
<li><span class="text-muted-foreground px-2">/</span></li>
|
||||
<li><span class="font-medium" x-text="reportId"></span></li>
|
||||
</ol>
|
||||
</nav>
|
||||
|
||||
<!-- Loading state -->
|
||||
<template x-if="loading">
|
||||
<div class="flex items-center justify-center py-16">
|
||||
<span class="loading loading-spinner loading-lg"></span>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<!-- Error state -->
|
||||
<template x-if="!loading && error">
|
||||
<div class="alert alert-error mb-6">
|
||||
<svg xmlns="http://www.w3.org/2000/svg" class="stroke-current shrink-0 h-6 w-6" fill="none" viewBox="0 0 24 24">
|
||||
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M10 14l2-2m0 0l2-2m-2 2l-2-2m2 2l2 2m7-2a9 9 0 11-18 0 9 9 0 0118 0z" />
|
||||
</svg>
|
||||
<span x-text="error"></span>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<!-- Report content -->
|
||||
<template x-if="!loading && !error && report">
|
||||
<div class="grid grid-cols-1 gap-6">
|
||||
<!-- Header -->
|
||||
<div class="flex justify-between items-start">
|
||||
<div>
|
||||
<h1 class="text-2xl font-bold mb-1" x-text="'Report: ' + report.report_id"></h1>
|
||||
<p class="text-muted-foreground">
|
||||
Submitted by <span x-text="report.org_name"></span>
|
||||
•
|
||||
<a :href="'/domains/' + report.domain" class="hover:text-primary" x-text="report.domain"></a>
|
||||
</p>
|
||||
</div>
|
||||
<div class="flex space-x-2">
|
||||
<a :href="'/domains/' + report.domain" class="btn btn-outline btn-sm">
|
||||
Back to Domain
|
||||
</a>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- Summary Cards -->
|
||||
<div class="grid grid-cols-1 sm:grid-cols-2 lg:grid-cols-4 gap-4">
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Total Emails{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
<div class="stat-value text-2xl font-bold" x-text="report.summary.total_count">-</div>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Passed{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
<div class="stat-value text-2xl font-bold text-success" x-text="report.summary.passed_count">-</div>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Failed{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
<div class="stat-value text-2xl font-bold text-error" x-text="report.summary.failed_count">-</div>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Pass Rate{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
<div class="stat-value text-2xl font-bold"
|
||||
:class="passRateClass(report.summary.pass_rate)"
|
||||
x-text="report.summary.pass_rate.toFixed(1) + '%'">-</div>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
</div>
|
||||
|
||||
<!-- Report Metadata -->
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Report Metadata{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
<div class="grid grid-cols-1 sm:grid-cols-2 gap-4">
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Report ID</p>
|
||||
<p class="font-medium font-mono text-sm break-all" x-text="report.report_id"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Organization</p>
|
||||
<p class="font-medium" x-text="report.org_name"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Email</p>
|
||||
<p class="font-medium" x-text="report.email || '—'"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Domain</p>
|
||||
<p class="font-medium" x-text="report.domain"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Period Start</p>
|
||||
<p class="font-medium" x-text="formatDate(report.begin_timestamp)"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Period End</p>
|
||||
<p class="font-medium" x-text="formatDate(report.end_timestamp)"></p>
|
||||
</div>
|
||||
</div>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
|
||||
<!-- Policy Published -->
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Policy Published{% endcall %}
|
||||
{% call card_description() %}DMARC policy in effect during the reporting period{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
<div class="grid grid-cols-1 sm:grid-cols-3 gap-4">
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Policy (p)</p>
|
||||
<p class="font-medium capitalize" x-text="report.policy.p"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Subdomain Policy (sp)</p>
|
||||
<p class="font-medium capitalize" x-text="report.policy.sp || '—'"></p>
|
||||
</div>
|
||||
<div>
|
||||
<p class="text-sm text-muted-foreground">Percentage (pct)</p>
|
||||
<p class="font-medium" x-text="report.policy.pct + '%'"></p>
|
||||
</div>
|
||||
</div>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
|
||||
<!-- Records -->
|
||||
{% call card() %}
|
||||
{% call card_header() %}
|
||||
{% call card_title() %}Records{% endcall %}
|
||||
{% call card_description() %}
|
||||
<span x-text="report.records.length"></span> IP source(s) in this report
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% call card_content() %}
|
||||
{% call table() %}
|
||||
{% call thead() %}
|
||||
{% call tr() %}
|
||||
{% call th() %}Source IP{% endcall %}
|
||||
{% call th() %}Count{% endcall %}
|
||||
{% call th() %}Disposition{% endcall %}
|
||||
{% call th() %}DKIM{% endcall %}
|
||||
{% call th() %}SPF{% endcall %}
|
||||
{% call th() %}Header From{% endcall %}
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% call tbody() %}
|
||||
<template x-if="report.records.length === 0">
|
||||
<tr>
|
||||
<td colspan="6" class="text-center py-4">
|
||||
<div class="text-muted-foreground">No records in this report</div>
|
||||
</td>
|
||||
</tr>
|
||||
</template>
|
||||
<template x-for="(record, index) in report.records" :key="index">
|
||||
{% call tr() %}
|
||||
{% call td() %}
|
||||
<span class="font-mono text-sm" x-text="record.source_ip"></span>
|
||||
{% endcall %}
|
||||
{% call td() %}
|
||||
<span x-text="record.count"></span>
|
||||
{% endcall %}
|
||||
{% call td() %}
|
||||
<span class="inline-flex items-center px-2 py-1 rounded text-xs capitalize"
|
||||
:class="dispositionClass(record.disposition)"
|
||||
x-text="record.disposition"></span>
|
||||
{% endcall %}
|
||||
{% call td() %}
|
||||
<span class="inline-flex items-center px-2 py-1 rounded text-xs capitalize"
|
||||
:class="resultClass(record.dkim_result)"
|
||||
x-text="record.dkim_result || '—'"></span>
|
||||
{% endcall %}
|
||||
{% call td() %}
|
||||
<span class="inline-flex items-center px-2 py-1 rounded text-xs capitalize"
|
||||
:class="resultClass(record.spf_result)"
|
||||
x-text="record.spf_result || '—'"></span>
|
||||
{% endcall %}
|
||||
{% call td() %}
|
||||
<span x-text="record.header_from || '—'"></span>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
</template>
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
{% endcall %}
|
||||
</div>
|
||||
</template>
|
||||
</div>
|
||||
{% endblock %}
|
||||
|
||||
{% block scripts %}
|
||||
<script>
|
||||
function reportDetailApp(reportId) {
|
||||
return {
|
||||
reportId: reportId,
|
||||
report: null,
|
||||
loading: true,
|
||||
error: null,
|
||||
|
||||
async init() {
|
||||
await this.fetchReport();
|
||||
},
|
||||
|
||||
async fetchReport() {
|
||||
try {
|
||||
const response = await fetch(`/api/v1/reports/${encodeURIComponent(this.reportId)}`);
|
||||
if (response.ok) {
|
||||
this.report = await response.json();
|
||||
} else if (response.status === 404) {
|
||||
this.error = `Report '${this.reportId}' was not found. It may have been deleted or may not exist.`;
|
||||
} else {
|
||||
this.error = 'Failed to load report. Please try again later.';
|
||||
}
|
||||
} catch (err) {
|
||||
this.error = 'Network error — could not load report.';
|
||||
console.error('Error fetching report:', err);
|
||||
} finally {
|
||||
this.loading = false;
|
||||
}
|
||||
},
|
||||
|
||||
formatDate(timestamp) {
|
||||
if (!timestamp) return '—';
|
||||
return new Date(timestamp * 1000).toLocaleString();
|
||||
},
|
||||
|
||||
passRateClass(rate) {
|
||||
if (rate >= 90) return 'text-success';
|
||||
if (rate >= 50) return 'text-warning';
|
||||
return 'text-error';
|
||||
},
|
||||
|
||||
resultClass(result) {
|
||||
if (result === 'pass') return 'bg-green-100 text-green-800';
|
||||
if (result === 'fail') return 'bg-red-100 text-red-800';
|
||||
return 'bg-gray-100 text-gray-800';
|
||||
},
|
||||
|
||||
dispositionClass(disposition) {
|
||||
if (disposition === 'none') return 'bg-green-100 text-green-800';
|
||||
if (disposition === 'quarantine') return 'bg-yellow-100 text-yellow-800';
|
||||
if (disposition === 'reject') return 'bg-red-100 text-red-800';
|
||||
return 'bg-gray-100 text-gray-800';
|
||||
},
|
||||
};
|
||||
}
|
||||
</script>
|
||||
{% endblock %}
|
||||
@@ -132,3 +132,44 @@ def test_upload_after_delete_succeeds(client: TestClient):
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["success"] is True
|
||||
|
||||
|
||||
def test_get_report_by_id_returns_detail(client: TestClient):
|
||||
"""GET /api/v1/reports/{report_id} returns full report detail after upload."""
|
||||
zip_bytes = _make_zip(SAMPLE_XML)
|
||||
client.post(
|
||||
"/api/v1/reports/upload",
|
||||
files={"file": ("report.zip", zip_bytes, "application/zip")},
|
||||
)
|
||||
|
||||
response = client.get("/api/v1/reports/123456789")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["report_id"] == "123456789"
|
||||
assert data["domain"] == "example.com"
|
||||
assert data["org_name"] == "google.com"
|
||||
assert "policy" in data
|
||||
assert "records" in data
|
||||
assert "summary" in data
|
||||
assert data["summary"]["total_count"] == 2
|
||||
|
||||
|
||||
def test_get_report_by_id_not_found(client: TestClient):
|
||||
"""GET /api/v1/reports/{report_id} returns 404 when report does not exist."""
|
||||
response = client.get("/api/v1/reports/no-such-report-id")
|
||||
assert response.status_code == 404
|
||||
|
||||
|
||||
def test_report_detail_html_page():
|
||||
"""GET /reports/{report_id} returns 200 HTML page.
|
||||
|
||||
The /reports/{report_id} route is registered on the module-level ``app``
|
||||
instance in main.py, not on the ``create_app()`` instance used by the
|
||||
``client`` fixture, so we must import the module-level app here.
|
||||
"""
|
||||
from app.main import app as main_app # noqa: PLC0415
|
||||
|
||||
with TestClient(main_app) as c:
|
||||
response = c.get("/reports/123456789")
|
||||
assert response.status_code == 200
|
||||
assert "text/html" in response.headers["content-type"]
|
||||
|
||||
Reference in New Issue
Block a user