diff --git a/api/openapi.yaml b/api/openapi.yaml index a44a588..c78c71b 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -240,7 +240,6 @@ components: - test_id - score - grade - - checks - created_at properties: id: @@ -264,10 +263,6 @@ components: example: "A" summary: $ref: '#/components/schemas/ScoreSummary' - checks: - type: array - items: - $ref: '#/components/schemas/Check' authentication: $ref: '#/components/schemas/AuthenticationResults' spamassassin: @@ -289,6 +284,10 @@ components: listed: false - rbl: "bl.spamcop.net" listed: false + content_analysis: + $ref: '#/components/schemas/ContentAnalysis' + header_analysis: + $ref: '#/components/schemas/HeaderAnalysis' raw_headers: type: string description: Raw email headers @@ -336,55 +335,272 @@ components: description: Header quality score (in percentage) example: 9 - Check: + ContentAnalysis: + type: object + properties: + has_html: + type: boolean + description: Whether email contains HTML part + example: true + has_plaintext: + type: boolean + description: Whether email contains plaintext part + example: true + html_issues: + type: array + items: + $ref: '#/components/schemas/ContentIssue' + description: Issues found in HTML content + links: + type: array + items: + $ref: '#/components/schemas/LinkCheck' + description: Analysis of links found in the email + images: + type: array + items: + $ref: '#/components/schemas/ImageCheck' + description: Analysis of images in the email + text_to_image_ratio: + type: number + format: float + description: Ratio of text to images (higher is better) + example: 0.75 + has_unsubscribe_link: + type: boolean + description: Whether email contains an unsubscribe link + example: true + unsubscribe_methods: + type: array + items: + type: string + enum: [link, mailto, list-unsubscribe-header, one-click] + description: Available unsubscribe methods + example: ["link", "list-unsubscribe-header"] + + ContentIssue: type: object required: - - category - - name - - status - - score - - grade + - type + - severity - message properties: - category: + type: type: string - enum: [authentication, dns, content, blacklist, headers, spam] - description: Check category - example: "authentication" - name: - type: string - description: Check name - example: "DKIM Signature" - status: - type: string - enum: [pass, fail, warn, info, error] - description: Check result status - example: "pass" - score: - type: integer - description: Points contributed to total score - example: 10 - grade: - type: string - enum: [A+, A, B, C, D, E, F] - description: Letter grade representation of the score (A+ is best, F is worst) - example: "A" - message: - type: string - description: Human-readable result message - example: "DKIM signature is valid" - details: - type: string - description: Additional details (may be JSON) + enum: [broken_html, missing_alt, excessive_images, obfuscated_url, suspicious_link] + description: Type of content issue + example: "missing_alt" severity: type: string enum: [critical, high, medium, low, info] description: Issue severity - example: "info" + example: "medium" + message: + type: string + description: Human-readable description + example: "3 images are missing alt attributes" + location: + type: string + description: Where the issue was found + example: "HTML body line 42" advice: type: string - description: Remediation advice - example: "Your DKIM configuration is correct" + description: How to fix this issue + example: "Add descriptive alt text to all images for better accessibility and deliverability" + + LinkCheck: + type: object + required: + - url + - status + properties: + url: + type: string + format: uri + description: The URL found in the email + example: "https://example.com/page" + status: + type: string + enum: [valid, broken, suspicious, redirected, timeout] + description: Link validation status + example: "valid" + http_code: + type: integer + description: HTTP status code received + example: 200 + redirect_chain: + type: array + items: + type: string + description: URLs in the redirect chain, if any + example: ["https://example.com", "https://www.example.com"] + is_shortened: + type: boolean + description: Whether this is a URL shortener + example: false + + ImageCheck: + type: object + required: + - has_alt + properties: + src: + type: string + description: Image source URL or path + example: "https://example.com/logo.png" + has_alt: + type: boolean + description: Whether image has alt attribute + example: true + alt_text: + type: string + description: Alt text content + example: "Company Logo" + is_tracking_pixel: + type: boolean + description: Whether this appears to be a tracking pixel (1x1 image) + example: false + + HeaderAnalysis: + type: object + properties: + has_mime_structure: + type: boolean + description: Whether body has a MIME structure + example: true + headers: + type: object + additionalProperties: + $ref: '#/components/schemas/HeaderCheck' + description: Map of header names to their check results (e.g., "from", "to", "dkim-signature") + example: + from: + present: true + value: "sender@example.com" + valid: true + importance: "required" + date: + present: true + value: "Mon, 1 Jan 2024 12:00:00 +0000" + valid: true + importance: "required" + received_chain: + type: array + items: + $ref: '#/components/schemas/ReceivedHop' + description: Chain of Received headers showing email path + domain_alignment: + $ref: '#/components/schemas/DomainAlignment' + issues: + type: array + items: + $ref: '#/components/schemas/HeaderIssue' + description: Issues found in headers + + HeaderCheck: + type: object + required: + - present + properties: + present: + type: boolean + description: Whether the header is present + example: true + value: + type: string + description: Header value + example: "sender@example.com" + valid: + type: boolean + description: Whether the value is valid/well-formed + example: true + importance: + type: string + enum: [required, recommended, optional] + description: How important this header is for deliverability + example: "required" + issues: + type: array + items: + type: string + description: Any issues with this header + example: ["Invalid date format"] + + ReceivedHop: + type: object + properties: + from: + type: string + description: Sending server hostname + example: "mail.example.com" + by: + type: string + description: Receiving server hostname + example: "mx.receiver.com" + with: + type: string + description: Protocol used + example: "ESMTPS" + id: + type: string + description: Message ID at this hop + timestamp: + type: string + format: date-time + description: When this hop occurred + + DomainAlignment: + type: object + properties: + from_domain: + type: string + description: Domain from From header + example: "example.com" + return_path_domain: + type: string + description: Domain from Return-Path header + example: "example.com" + dkim_domains: + type: array + items: + type: string + description: Domains from DKIM signatures + example: ["example.com"] + aligned: + type: boolean + description: Whether all domains align + example: true + issues: + type: array + items: + type: string + description: Alignment issues + example: ["Return-Path domain does not match From domain"] + + HeaderIssue: + type: object + required: + - header + - severity + - message + properties: + header: + type: string + description: Header name + example: "Date" + severity: + type: string + enum: [critical, high, medium, low, info] + description: Issue severity + example: "medium" + message: + type: string + description: Human-readable description + example: "Date header is in the future" + advice: + type: string + description: How to fix this issue + example: "Ensure your mail server clock is synchronized with NTP" AuthenticationResults: type: object @@ -522,6 +738,9 @@ components: type: string description: RBL response code or message example: "127.0.0.2" + error: + type: string + description: RBL error if any Status: type: object diff --git a/internal/app/cli_analyzer.go b/internal/app/cli_analyzer.go index 03a1720..4334711 100644 --- a/internal/app/cli_analyzer.go +++ b/internal/app/cli_analyzer.go @@ -31,7 +31,6 @@ import ( "github.com/google/uuid" - "git.happydns.org/happyDeliver/internal/api" "git.happydns.org/happyDeliver/internal/config" "git.happydns.org/happyDeliver/pkg/analyzer" ) @@ -97,42 +96,7 @@ func outputHumanReadable(result *analyzer.AnalysisResult, emailAnalyzer *analyze fmt.Fprintln(writer, "DETAILED CHECK RESULTS") fmt.Fprintln(writer, strings.Repeat("-", 70)) - // Group checks by category - categories := make(map[api.CheckCategory][]api.Check) - for _, check := range result.Report.Checks { - categories[check.Category] = append(categories[check.Category], check) - } - - // Print checks by category - categoryOrder := []api.CheckCategory{ - api.Authentication, - api.Dns, - api.Blacklist, - api.Content, - api.Headers, - } - - for _, category := range categoryOrder { - checks, ok := categories[category] - if !ok || len(checks) == 0 { - continue - } - - fmt.Fprintf(writer, "\n%s:\n", category) - for _, check := range checks { - statusSymbol := "✓" - if check.Status == api.CheckStatusFail { - statusSymbol = "✗" - } else if check.Status == api.CheckStatusWarn { - statusSymbol = "⚠" - } - - fmt.Fprintf(writer, " %s %s: %s\n", statusSymbol, check.Name, check.Message) - if check.Advice != nil && *check.Advice != "" { - fmt.Fprintf(writer, " → %s\n", *check.Advice) - } - } - } + // TODO fmt.Fprintln(writer, "\n"+strings.Repeat("=", 70)) return nil diff --git a/pkg/analyzer/authentication.go b/pkg/analyzer/authentication.go index eef44b1..310c8f6 100644 --- a/pkg/analyzer/authentication.go +++ b/pkg/analyzer/authentication.go @@ -191,7 +191,7 @@ func (a *AuthenticationAnalyzer) parseDKIMResult(part string) *api.AuthResult { result.Selector = &selector } - result.Details = &part + result.Details = api.PtrTo(strings.TrimPrefix(part, "dkim=")) return result } @@ -215,7 +215,7 @@ func (a *AuthenticationAnalyzer) parseDMARCResult(part string) *api.AuthResult { result.Domain = &domain } - result.Details = &part + result.Details = api.PtrTo(strings.TrimPrefix(part, "dmarc=")) return result } @@ -246,7 +246,7 @@ func (a *AuthenticationAnalyzer) parseBIMIResult(part string) *api.AuthResult { result.Selector = &selector } - result.Details = &part + result.Details = api.PtrTo(strings.TrimPrefix(part, "bimi=")) return result } @@ -263,7 +263,7 @@ func (a *AuthenticationAnalyzer) parseARCResult(part string) *api.ARCResult { result.Result = api.ARCResultResult(resultStr) } - result.Details = &part + result.Details = api.PtrTo(strings.TrimPrefix(part, "arc=")) return result } @@ -467,3 +467,75 @@ func textprotoCanonical(s string) string { } return strings.Join(words, "-") } + +// CalculateAuthenticationScore calculates the authentication score from auth results +// Returns a score from 0-100 where higher is better +func (a *AuthenticationAnalyzer) CalculateAuthenticationScore(results *api.AuthenticationResults) int { + if results == nil { + return 0 + } + + score := 0 + + // SPF (30 points) + if results.Spf != nil { + switch results.Spf.Result { + case api.AuthResultResultPass: + score += 30 + case api.AuthResultResultNeutral, api.AuthResultResultNone: + score += 15 + case api.AuthResultResultSoftfail: + score += 5 + default: // fail, temperror, permerror + score += 0 + } + } + + // DKIM (30 points) - at least one passing signature + if results.Dkim != nil && len(*results.Dkim) > 0 { + hasPass := false + for _, dkim := range *results.Dkim { + if dkim.Result == api.AuthResultResultPass { + hasPass = true + break + } + } + if hasPass { + score += 30 + } else { + // Has DKIM signatures but none passed + score += 10 + } + } + + // DMARC (30 points) + if results.Dmarc != nil { + switch results.Dmarc.Result { + case api.AuthResultResultPass: + score += 30 + case api.AuthResultResultNone: + score += 10 + default: // fail + score += 0 + } + } + + // BIMI (10 points) + if results.Bimi != nil { + switch results.Bimi.Result { + case api.AuthResultResultPass: + score += 10 + case api.AuthResultResultNone: + score += 5 + default: // fail + score += 0 + } + } + + // Ensure score doesn't exceed 100 + if score > 100 { + score = 100 + } + + return score +} diff --git a/pkg/analyzer/authentication_checks.go b/pkg/analyzer/authentication_checks.go deleted file mode 100644 index f7cc15e..0000000 --- a/pkg/analyzer/authentication_checks.go +++ /dev/null @@ -1,317 +0,0 @@ -// This file is part of the happyDeliver (R) project. -// Copyright (c) 2025 happyDomain -// Authors: Pierre-Olivier Mercier, et al. -// -// This program is offered under a commercial and under the AGPL license. -// For commercial licensing, contact us at . -// -// For AGPL licensing: -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package analyzer - -import ( - "fmt" - "strings" - - "git.happydns.org/happyDeliver/internal/api" -) - -// GenerateAuthenticationChecks generates check results for authentication -func (a *AuthenticationAnalyzer) GenerateAuthenticationChecks(results *api.AuthenticationResults) []api.Check { - var checks []api.Check - - // SPF check - if results.Spf != nil { - check := a.generateSPFCheck(results.Spf) - checks = append(checks, check) - } else { - checks = append(checks, api.Check{ - Category: api.Authentication, - Name: "SPF Record", - Status: api.CheckStatusWarn, - Score: 0, - Message: "No SPF authentication result found", - Severity: api.PtrTo(api.CheckSeverityMedium), - Advice: api.PtrTo("Ensure your MTA is configured to check SPF records"), - }) - } - - // DKIM check - if results.Dkim != nil && len(*results.Dkim) > 0 { - for i, dkim := range *results.Dkim { - check := a.generateDKIMCheck(&dkim, i) - checks = append(checks, check) - } - } else { - checks = append(checks, api.Check{ - Category: api.Authentication, - Name: "DKIM Signature", - Status: api.CheckStatusWarn, - Score: 0, - Message: "No DKIM signature found", - Severity: api.PtrTo(api.CheckSeverityMedium), - Advice: api.PtrTo("Configure DKIM signing for your domain to improve deliverability"), - }) - } - - // DMARC check - if results.Dmarc != nil { - check := a.generateDMARCCheck(results.Dmarc) - checks = append(checks, check) - } else { - checks = append(checks, api.Check{ - Category: api.Authentication, - Name: "DMARC Policy", - Status: api.CheckStatusWarn, - Score: 0, - Message: "No DMARC authentication result found", - Severity: api.PtrTo(api.CheckSeverityMedium), - Advice: api.PtrTo("Implement DMARC policy for your domain"), - }) - } - - // BIMI check (optional, informational only) - if results.Bimi != nil { - check := a.generateBIMICheck(results.Bimi) - checks = append(checks, check) - } - - // ARC check (optional, for forwarded emails) - if results.Arc != nil { - check := a.generateARCCheck(results.Arc) - checks = append(checks, check) - } - - return checks -} - -func (a *AuthenticationAnalyzer) generateSPFCheck(spf *api.AuthResult) api.Check { - check := api.Check{ - Category: api.Authentication, - Name: "SPF Record", - } - - switch spf.Result { - case api.AuthResultResultPass: - check.Status = api.CheckStatusPass - check.Score = 100 - check.Message = "SPF validation passed" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - case api.AuthResultResultFail: - check.Status = api.CheckStatusFail - check.Score = 0 - check.Message = "SPF validation failed" - check.Severity = api.PtrTo(api.CheckSeverityCritical) - check.Advice = api.PtrTo("Fix your SPF record to authorize this sending server") - case api.AuthResultResultSoftfail: - check.Status = api.CheckStatusWarn - check.Score = 50 - check.Message = "SPF validation softfail" - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("Review your SPF record configuration") - case api.AuthResultResultNeutral: - check.Status = api.CheckStatusWarn - check.Score = 50 - check.Message = "SPF validation neutral" - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Advice = api.PtrTo("Consider tightening your SPF policy") - default: - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Message = fmt.Sprintf("SPF validation result: %s", spf.Result) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("Review your SPF record configuration") - } - - if spf.Details != nil { - check.Details = spf.Details - } else if spf.Domain != nil { - details := fmt.Sprintf("Domain: %s", *spf.Domain) - check.Details = &details - } - - return check -} - -func (a *AuthenticationAnalyzer) generateDKIMCheck(dkim *api.AuthResult, index int) api.Check { - check := api.Check{ - Category: api.Authentication, - Name: fmt.Sprintf("DKIM Signature #%d", index+1), - } - - switch dkim.Result { - case api.AuthResultResultPass: - check.Status = api.CheckStatusPass - check.Score = 10 - check.Message = "DKIM signature is valid" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("Your DKIM signature is properly configured") - case api.AuthResultResultFail: - check.Status = api.CheckStatusFail - check.Score = 0 - check.Message = "DKIM signature validation failed" - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Check your DKIM keys and signing configuration") - default: - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Message = fmt.Sprintf("DKIM validation result: %s", dkim.Result) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("Ensure DKIM signing is enabled and configured correctly") - } - - if dkim.Details != nil { - check.Details = dkim.Details - } else { - var detailsParts []string - if dkim.Domain != nil { - detailsParts = append(detailsParts, fmt.Sprintf("Domain: %s", *dkim.Domain)) - } - if dkim.Selector != nil { - detailsParts = append(detailsParts, fmt.Sprintf("Selector: %s", *dkim.Selector)) - } - if len(detailsParts) > 0 { - details := strings.Join(detailsParts, ", ") - check.Details = &details - } - } - - return check -} - -func (a *AuthenticationAnalyzer) generateDMARCCheck(dmarc *api.AuthResult) api.Check { - check := api.Check{ - Category: api.Authentication, - Name: "DMARC Policy", - } - - switch dmarc.Result { - case api.AuthResultResultPass: - check.Status = api.CheckStatusPass - check.Score = 10 - check.Message = "DMARC validation passed" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("Your DMARC policy is properly aligned") - case api.AuthResultResultFail: - check.Status = api.CheckStatusFail - check.Score = 0 - check.Message = "DMARC validation failed" - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Ensure SPF or DKIM alignment with your From domain") - default: - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Message = fmt.Sprintf("DMARC validation result: %s", dmarc.Result) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("Configure DMARC policy for your domain") - } - - if dmarc.Details != nil { - check.Details = dmarc.Details - } else if dmarc.Domain != nil { - details := fmt.Sprintf("Domain: %s", *dmarc.Domain) - check.Details = &details - } - - return check -} - -func (a *AuthenticationAnalyzer) generateBIMICheck(bimi *api.AuthResult) api.Check { - check := api.Check{ - Category: api.Authentication, - Name: "BIMI (Brand Indicators)", - } - - switch bimi.Result { - case api.AuthResultResultPass: - check.Status = api.CheckStatusPass - check.Score = 0 // BIMI doesn't contribute to score (branding feature) - check.Message = "BIMI validation passed" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("Your brand logo is properly configured via BIMI") - case api.AuthResultResultFail: - check.Status = api.CheckStatusInfo - check.Score = 0 - check.Message = "BIMI validation failed" - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Advice = api.PtrTo("BIMI is optional but can improve brand recognition. Ensure DMARC is enforced (p=quarantine or p=reject) and configure a valid BIMI record") - default: - check.Status = api.CheckStatusInfo - check.Score = 0 - check.Message = fmt.Sprintf("BIMI validation result: %s", bimi.Result) - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Advice = api.PtrTo("BIMI is optional. Consider implementing it to display your brand logo in supported email clients") - } - - if bimi.Details != nil { - check.Details = bimi.Details - } else if bimi.Domain != nil { - details := fmt.Sprintf("Domain: %s", *bimi.Domain) - check.Details = &details - } - - return check -} - -func (a *AuthenticationAnalyzer) generateARCCheck(arc *api.ARCResult) api.Check { - check := api.Check{ - Category: api.Authentication, - Name: "ARC (Authenticated Received Chain)", - } - - switch arc.Result { - case api.ARCResultResultPass: - check.Status = api.CheckStatusPass - check.Score = 0 // ARC doesn't contribute to score (informational for forwarding) - check.Message = "ARC chain validation passed" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("ARC preserves authentication results through email forwarding. Your email passed through intermediaries while maintaining authentication") - case api.ARCResultResultFail: - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Message = "ARC chain validation failed" - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("The ARC chain is broken or invalid. This may indicate issues with email forwarding intermediaries") - default: - check.Status = api.CheckStatusInfo - check.Score = 0 - check.Message = "No ARC chain present" - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Advice = api.PtrTo("ARC is not present. This is normal for emails sent directly without forwarding through mailing lists or other intermediaries") - } - - if arc.Details != nil { - check.Details = arc.Details - } else { - // Build details - var detailsParts []string - if arc.ChainLength != nil { - detailsParts = append(detailsParts, fmt.Sprintf("Chain length: %d", *arc.ChainLength)) - } - if arc.ChainValid != nil { - detailsParts = append(detailsParts, fmt.Sprintf("Chain valid: %v", *arc.ChainValid)) - } - if arc.Details != nil { - detailsParts = append(detailsParts, *arc.Details) - } - - if len(detailsParts) > 0 { - details := strings.Join(detailsParts, ", ") - check.Details = &details - } - } - - return check -} diff --git a/pkg/analyzer/authentication_test.go b/pkg/analyzer/authentication_test.go index 0b03998..e7f1e06 100644 --- a/pkg/analyzer/authentication_test.go +++ b/pkg/analyzer/authentication_test.go @@ -22,7 +22,6 @@ package analyzer import ( - "strings" "testing" "git.happydns.org/happyDeliver/internal/api" @@ -246,250 +245,6 @@ func TestParseBIMIResult(t *testing.T) { } } -func TestGenerateAuthSPFCheck(t *testing.T) { - tests := []struct { - name string - spf *api.AuthResult - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "SPF pass", - spf: &api.AuthResult{ - Result: api.AuthResultResultPass, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "SPF fail", - spf: &api.AuthResult{ - Result: api.AuthResultResultFail, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - { - name: "SPF softfail", - spf: &api.AuthResult{ - Result: api.AuthResultResultSoftfail, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 5, - }, - { - name: "SPF neutral", - spf: &api.AuthResult{ - Result: api.AuthResultResultNeutral, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 5, - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateSPFCheck(tt.spf) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Authentication { - t.Errorf("Category = %v, want %v", check.Category, api.Authentication) - } - if check.Name != "SPF Record" { - t.Errorf("Name = %q, want %q", check.Name, "SPF Record") - } - }) - } -} - -func TestGenerateAuthDKIMCheck(t *testing.T) { - tests := []struct { - name string - dkim *api.AuthResult - index int - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "DKIM pass", - dkim: &api.AuthResult{ - Result: api.AuthResultResultPass, - Domain: api.PtrTo("example.com"), - Selector: api.PtrTo("default"), - }, - index: 0, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "DKIM fail", - dkim: &api.AuthResult{ - Result: api.AuthResultResultFail, - Domain: api.PtrTo("example.com"), - Selector: api.PtrTo("default"), - }, - index: 0, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - { - name: "DKIM none", - dkim: &api.AuthResult{ - Result: api.AuthResultResultNone, - Domain: api.PtrTo("example.com"), - Selector: api.PtrTo("default"), - }, - index: 0, - expectedStatus: api.CheckStatusWarn, - expectedScore: 0, - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateDKIMCheck(tt.dkim, tt.index) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Authentication { - t.Errorf("Category = %v, want %v", check.Category, api.Authentication) - } - if !strings.Contains(check.Name, "DKIM Signature") { - t.Errorf("Name should contain 'DKIM Signature', got %q", check.Name) - } - }) - } -} - -func TestGenerateAuthDMARCCheck(t *testing.T) { - tests := []struct { - name string - dmarc *api.AuthResult - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "DMARC pass", - dmarc: &api.AuthResult{ - Result: api.AuthResultResultPass, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "DMARC fail", - dmarc: &api.AuthResult{ - Result: api.AuthResultResultFail, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateDMARCCheck(tt.dmarc) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Authentication { - t.Errorf("Category = %v, want %v", check.Category, api.Authentication) - } - if check.Name != "DMARC Policy" { - t.Errorf("Name = %q, want %q", check.Name, "DMARC Policy") - } - }) - } -} - -func TestGenerateAuthBIMICheck(t *testing.T) { - tests := []struct { - name string - bimi *api.AuthResult - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "BIMI pass", - bimi: &api.AuthResult{ - Result: api.AuthResultResultPass, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 0, // BIMI doesn't contribute to score - }, - { - name: "BIMI fail", - bimi: &api.AuthResult{ - Result: api.AuthResultResultFail, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusInfo, - expectedScore: 0, - }, - { - name: "BIMI none", - bimi: &api.AuthResult{ - Result: api.AuthResultResultNone, - Domain: api.PtrTo("example.com"), - }, - expectedStatus: api.CheckStatusInfo, - expectedScore: 0, - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateBIMICheck(tt.bimi) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Authentication { - t.Errorf("Category = %v, want %v", check.Category, api.Authentication) - } - if check.Name != "BIMI (Brand Indicators)" { - t.Errorf("Name = %q, want %q", check.Name, "BIMI (Brand Indicators)") - } - - // BIMI should always have score of 0.0 (branding feature) - if check.Score != 0.0 { - t.Error("BIMI should not contribute to deliverability score") - } - }) - } -} - func TestGetAuthenticationScore(t *testing.T) { tests := []struct { name string @@ -563,11 +318,11 @@ func TestGetAuthenticationScore(t *testing.T) { }, } - scorer := NewDeliverabilityScorer() + scorer := NewAuthenticationAnalyzer() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - score := scorer.GetAuthenticationScore(tt.results) + score := scorer.CalculateAuthenticationScore(tt.results) if score != tt.expectedScore { t.Errorf("Score = %v, want %v", score, tt.expectedScore) @@ -576,92 +331,6 @@ func TestGetAuthenticationScore(t *testing.T) { } } -func TestGenerateAuthenticationChecks(t *testing.T) { - tests := []struct { - name string - results *api.AuthenticationResults - expectedChecks int - }{ - { - name: "All authentication methods present", - results: &api.AuthenticationResults{ - Spf: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - Dkim: &[]api.AuthResult{ - {Result: api.AuthResultResultPass}, - }, - Dmarc: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - Bimi: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - }, - expectedChecks: 4, // SPF, DKIM, DMARC, BIMI - }, - { - name: "Without BIMI", - results: &api.AuthenticationResults{ - Spf: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - Dkim: &[]api.AuthResult{ - {Result: api.AuthResultResultPass}, - }, - Dmarc: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - }, - expectedChecks: 3, // SPF, DKIM, DMARC - }, - { - name: "No authentication results", - results: &api.AuthenticationResults{}, - expectedChecks: 3, // SPF, DKIM, DMARC warnings for missing - }, - { - name: "With ARC", - results: &api.AuthenticationResults{ - Spf: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - Dkim: &[]api.AuthResult{ - {Result: api.AuthResultResultPass}, - }, - Dmarc: &api.AuthResult{ - Result: api.AuthResultResultPass, - }, - Arc: &api.ARCResult{ - Result: api.ARCResultResultPass, - ChainLength: api.PtrTo(2), - ChainValid: api.PtrTo(true), - }, - }, - expectedChecks: 4, // SPF, DKIM, DMARC, ARC - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := analyzer.GenerateAuthenticationChecks(tt.results) - - if len(checks) != tt.expectedChecks { - t.Errorf("Got %d checks, want %d", len(checks), tt.expectedChecks) - } - - // Verify all checks have the Authentication category - for _, check := range checks { - if check.Category != api.Authentication { - t.Errorf("Check %s has category %v, want %v", check.Name, check.Category, api.Authentication) - } - } - }) - } -} - func TestParseARCResult(t *testing.T) { tests := []struct { name string @@ -783,64 +452,3 @@ func TestValidateARCChain(t *testing.T) { }) } } - -func TestGenerateARCCheck(t *testing.T) { - tests := []struct { - name string - arc *api.ARCResult - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "ARC pass", - arc: &api.ARCResult{ - Result: api.ARCResultResultPass, - ChainLength: api.PtrTo(2), - ChainValid: api.PtrTo(true), - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 0, // ARC doesn't contribute to score - }, - { - name: "ARC fail", - arc: &api.ARCResult{ - Result: api.ARCResultResultFail, - ChainLength: api.PtrTo(1), - ChainValid: api.PtrTo(false), - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 0, - }, - { - name: "ARC none", - arc: &api.ARCResult{ - Result: api.ARCResultResultNone, - ChainLength: api.PtrTo(0), - ChainValid: api.PtrTo(true), - }, - expectedStatus: api.CheckStatusInfo, - expectedScore: 0, - }, - } - - analyzer := NewAuthenticationAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateARCCheck(tt.arc) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Authentication { - t.Errorf("Category = %v, want %v", check.Category, api.Authentication) - } - if !strings.Contains(check.Name, "ARC") { - t.Errorf("Name should contain 'ARC', got %q", check.Name) - } - }) - } -} diff --git a/pkg/analyzer/content.go b/pkg/analyzer/content.go index 872c75c..7964693 100644 --- a/pkg/analyzer/content.go +++ b/pkg/analyzer/content.go @@ -459,294 +459,151 @@ func (c *ContentAnalyzer) normalizeText(text string) string { return text } -// GenerateContentChecks generates check results for content analysis -func (c *ContentAnalyzer) GenerateContentChecks(results *ContentResults) []api.Check { - var checks []api.Check - +// GenerateContentAnalysis creates structured content analysis from results +func (c *ContentAnalyzer) GenerateContentAnalysis(results *ContentResults) *api.ContentAnalysis { if results == nil { - return checks + return nil } - // HTML validity check - checks = append(checks, c.generateHTMLValidityCheck(results)) - - // Link checks - checks = append(checks, c.generateLinkChecks(results)...) - - // Image checks - checks = append(checks, c.generateImageChecks(results)...) - - // Unsubscribe link check - checks = append(checks, c.generateUnsubscribeCheck(results)) - - // Text/HTML consistency check - if results.TextContent != "" && results.HTMLContent != "" { - checks = append(checks, c.generateTextConsistencyCheck(results)) + analysis := &api.ContentAnalysis{ + HasHtml: api.PtrTo(results.HTMLContent != ""), + HasPlaintext: api.PtrTo(results.TextContent != ""), + HasUnsubscribeLink: api.PtrTo(results.HasUnsubscribe), } - // Image-to-text ratio check + // Calculate text-to-image ratio (inverse of image-to-text) if len(results.Images) > 0 && results.HTMLContent != "" { - checks = append(checks, c.generateImageRatioCheck(results)) - } - - // Suspicious URLs check - if len(results.SuspiciousURLs) > 0 { - checks = append(checks, c.generateSuspiciousURLCheck(results)) - } - - return checks -} - -// generateHTMLValidityCheck creates a check for HTML validity -func (c *ContentAnalyzer) generateHTMLValidityCheck(results *ContentResults) api.Check { - check := api.Check{ - Category: api.Content, - Name: "HTML Structure", - } - - if !results.HTMLValid { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Message = "HTML structure is invalid" - if len(results.HTMLErrors) > 0 { - details := strings.Join(results.HTMLErrors, "; ") - check.Details = &details - } - check.Advice = api.PtrTo("Fix HTML structure errors to improve email rendering") - } else { - check.Status = api.CheckStatusPass - check.Score = 2 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "HTML structure is valid" - check.Advice = api.PtrTo("Your HTML is well-formed") - } - - return check -} - -// generateLinkChecks creates checks for links -func (c *ContentAnalyzer) generateLinkChecks(results *ContentResults) []api.Check { - var checks []api.Check - - if len(results.Links) == 0 { - return checks - } - - // Count broken links - brokenLinks := 0 - warningLinks := 0 - for _, link := range results.Links { - if link.Status >= 400 { - brokenLinks++ - } else if link.Warning != "" { - warningLinks++ + textLen := float32(len(c.extractTextFromHTML(results.HTMLContent))) + if textLen > 0 { + ratio := textLen / float32(len(results.Images)) + analysis.TextToImageRatio = &ratio } } - check := api.Check{ - Category: api.Content, - Name: "Links", - } + // Build HTML issues + htmlIssues := []api.ContentIssue{} - if brokenLinks > 0 { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Message = fmt.Sprintf("Found %d broken link(s)", brokenLinks) - check.Advice = api.PtrTo("Fix or remove broken links to improve deliverability") - details := fmt.Sprintf("Total links: %d, Broken: %d", len(results.Links), brokenLinks) - check.Details = &details - } else if warningLinks > 0 { - check.Status = api.CheckStatusWarn - check.Score = 3 - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = fmt.Sprintf("Found %d link(s) that could not be verified", warningLinks) - check.Advice = api.PtrTo("Review links that could not be verified") - details := fmt.Sprintf("Total links: %d, Unverified: %d", len(results.Links), warningLinks) - check.Details = &details - } else { - check.Status = api.CheckStatusPass - check.Score = 4 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = fmt.Sprintf("All %d link(s) are valid", len(results.Links)) - check.Advice = api.PtrTo("Your links are working properly") - } - - checks = append(checks, check) - return checks -} - -// generateImageChecks creates checks for images -func (c *ContentAnalyzer) generateImageChecks(results *ContentResults) []api.Check { - var checks []api.Check - - if len(results.Images) == 0 { - return checks - } - - // Count images without alt text - noAltCount := 0 - for _, img := range results.Images { - if !img.HasAlt { - noAltCount++ + // Add HTML parsing errors + if !results.HTMLValid && len(results.HTMLErrors) > 0 { + for _, errMsg := range results.HTMLErrors { + htmlIssues = append(htmlIssues, api.ContentIssue{ + Type: api.BrokenHtml, + Severity: api.ContentIssueSeverityHigh, + Message: errMsg, + Advice: api.PtrTo("Fix HTML structure errors to improve email rendering across clients"), + }) } } - check := api.Check{ - Category: api.Content, - Name: "Image Alt Attributes", + // Add missing alt text issues + if len(results.Images) > 0 { + missingAltCount := 0 + for _, img := range results.Images { + if !img.HasAlt { + missingAltCount++ + } + } + if missingAltCount > 0 { + htmlIssues = append(htmlIssues, api.ContentIssue{ + Type: api.MissingAlt, + Severity: api.ContentIssueSeverityMedium, + Message: fmt.Sprintf("%d image(s) missing alt attributes", missingAltCount), + Advice: api.PtrTo("Add descriptive alt text to all images for better accessibility and deliverability"), + }) + } } - if noAltCount == len(results.Images) { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Message = "No images have alt attributes" - check.Advice = api.PtrTo("Add alt text to all images for accessibility and deliverability") - details := fmt.Sprintf("Images without alt: %d/%d", noAltCount, len(results.Images)) - check.Details = &details - } else if noAltCount > 0 { - check.Status = api.CheckStatusWarn - check.Score = 2 - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = fmt.Sprintf("%d image(s) missing alt attributes", noAltCount) - check.Advice = api.PtrTo("Add alt text to all images for better accessibility") - details := fmt.Sprintf("Images without alt: %d/%d", noAltCount, len(results.Images)) - check.Details = &details - } else { - check.Status = api.CheckStatusPass - check.Score = 3 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "All images have alt attributes" - check.Advice = api.PtrTo("Your images are properly tagged for accessibility") + // Add excessive images issue + if results.ImageTextRatio > 10.0 { + htmlIssues = append(htmlIssues, api.ContentIssue{ + Type: api.ExcessiveImages, + Severity: api.ContentIssueSeverityMedium, + Message: "Email is excessively image-heavy", + Advice: api.PtrTo("Reduce the number of images relative to text content"), + }) } - checks = append(checks, check) - return checks + // Add suspicious URL issues + for _, suspURL := range results.SuspiciousURLs { + htmlIssues = append(htmlIssues, api.ContentIssue{ + Type: api.SuspiciousLink, + Severity: api.ContentIssueSeverityHigh, + Message: "Suspicious URL detected", + Location: &suspURL, + Advice: api.PtrTo("Avoid URL shorteners, IP addresses, and obfuscated URLs in emails"), + }) + } + + if len(htmlIssues) > 0 { + analysis.HtmlIssues = &htmlIssues + } + + // Convert links + if len(results.Links) > 0 { + links := make([]api.LinkCheck, 0, len(results.Links)) + for _, link := range results.Links { + status := api.Valid + if link.Status >= 400 { + status = api.Broken + } else if !link.IsSafe { + status = api.Suspicious + } else if link.Warning != "" { + status = api.Timeout + } + + apiLink := api.LinkCheck{ + Url: link.URL, + Status: status, + } + + if link.Status > 0 { + apiLink.HttpCode = api.PtrTo(link.Status) + } + + // Check if it's a URL shortener + parsedURL, err := url.Parse(link.URL) + if err == nil { + isShortened := c.isSuspiciousURL(link.URL, parsedURL) + apiLink.IsShortened = api.PtrTo(isShortened) + } + + links = append(links, apiLink) + } + analysis.Links = &links + } + + // Convert images + if len(results.Images) > 0 { + images := make([]api.ImageCheck, 0, len(results.Images)) + for _, img := range results.Images { + apiImg := api.ImageCheck{ + HasAlt: img.HasAlt, + } + if img.Src != "" { + apiImg.Src = &img.Src + } + if img.AltText != "" { + apiImg.AltText = &img.AltText + } + // Simple heuristic: tracking pixels are typically 1x1 + apiImg.IsTrackingPixel = api.PtrTo(false) + + images = append(images, apiImg) + } + analysis.Images = &images + } + + // Unsubscribe methods + if results.HasUnsubscribe { + methods := []api.ContentAnalysisUnsubscribeMethods{api.Link} + analysis.UnsubscribeMethods = &methods + } + + return analysis } -// generateUnsubscribeCheck creates a check for unsubscribe links -func (c *ContentAnalyzer) generateUnsubscribeCheck(results *ContentResults) api.Check { - check := api.Check{ - Category: api.Content, - Name: "Unsubscribe Link", - } - - if !results.HasUnsubscribe { - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = "No unsubscribe link found" - check.Advice = api.PtrTo("Add an unsubscribe link for marketing emails (RFC 8058)") - } else { - check.Status = api.CheckStatusPass - check.Score = 3 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = fmt.Sprintf("Found %d unsubscribe link(s)", len(results.UnsubscribeLinks)) - check.Advice = api.PtrTo("Your email includes an unsubscribe option") - } - - return check -} - -// generateTextConsistencyCheck creates a check for text/HTML consistency -func (c *ContentAnalyzer) generateTextConsistencyCheck(results *ContentResults) api.Check { - check := api.Check{ - Category: api.Content, - Name: "Plain Text Consistency", - } - - consistency := results.TextPlainRatio - - if consistency < 0.3 { - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = "Plain text and HTML versions differ significantly" - check.Advice = api.PtrTo("Ensure plain text and HTML versions convey the same content") - details := fmt.Sprintf("Consistency: %.0f%%", consistency*100) - check.Details = &details - } else { - check.Status = api.CheckStatusPass - check.Score = 3 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "Plain text and HTML versions are consistent" - check.Advice = api.PtrTo("Your multipart email is well-structured") - details := fmt.Sprintf("Consistency: %.0f%%", consistency*100) - check.Details = &details - } - - return check -} - -// generateImageRatioCheck creates a check for image-to-text ratio -func (c *ContentAnalyzer) generateImageRatioCheck(results *ContentResults) api.Check { - check := api.Check{ - Category: api.Content, - Name: "Image-to-Text Ratio", - } - - ratio := results.ImageTextRatio - - // Flag if more than 1 image per 100 characters (very image-heavy) - if ratio > 10.0 { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Message = "Email is excessively image-heavy" - check.Advice = api.PtrTo("Reduce the number of images relative to text content") - details := fmt.Sprintf("Images: %d, Ratio: %.2f images per 1000 chars", len(results.Images), ratio) - check.Details = &details - } else if ratio > 5.0 { - check.Status = api.CheckStatusWarn - check.Score = 2 - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = "Email has high image-to-text ratio" - check.Advice = api.PtrTo("Consider adding more text content relative to images") - details := fmt.Sprintf("Images: %d, Ratio: %.2f images per 1000 chars", len(results.Images), ratio) - check.Details = &details - } else { - check.Status = api.CheckStatusPass - check.Score = 3 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "Image-to-text ratio is reasonable" - check.Advice = api.PtrTo("Your content has a good balance of images and text") - details := fmt.Sprintf("Images: %d, Ratio: %.2f images per 1000 chars", len(results.Images), ratio) - check.Details = &details - } - - return check -} - -// generateSuspiciousURLCheck creates a check for suspicious URLs -func (c *ContentAnalyzer) generateSuspiciousURLCheck(results *ContentResults) api.Check { - check := api.Check{ - Category: api.Content, - Name: "Suspicious URLs", - } - - count := len(results.SuspiciousURLs) - - check.Status = api.CheckStatusWarn - check.Score = 0.0 - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Message = fmt.Sprintf("Found %d suspicious URL(s)", count) - check.Advice = api.PtrTo("Avoid URL shorteners, IP addresses, and obfuscated URLs in emails") - - if count <= 3 { - details := strings.Join(results.SuspiciousURLs, ", ") - check.Details = &details - } else { - details := fmt.Sprintf("%s, and %d more", strings.Join(results.SuspiciousURLs[:3], ", "), count-3) - check.Details = &details - } - - return check -} - -// GetContentScore calculates the content score (0-20 points) -func (c *ContentAnalyzer) GetContentScore(results *ContentResults) int { +// CalculateContentScore calculates the content score (0-20 points) +func (c *ContentAnalyzer) CalculateContentScore(results *ContentResults) int { if results == nil { return 0 } diff --git a/pkg/analyzer/content_test.go b/pkg/analyzer/content_test.go index 0a1c710..78a27e9 100644 --- a/pkg/analyzer/content_test.go +++ b/pkg/analyzer/content_test.go @@ -28,7 +28,6 @@ import ( "testing" "time" - "git.happydns.org/happyDeliver/internal/api" "golang.org/x/net/html" ) @@ -608,453 +607,6 @@ func TestAnalyzeContent_ImageAltAttributes(t *testing.T) { } } -func TestGenerateHTMLValidityCheck(t *testing.T) { - tests := []struct { - name string - results *ContentResults - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Valid HTML", - results: &ContentResults{ - HTMLValid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 2, - }, - { - name: "Invalid HTML", - results: &ContentResults{ - HTMLValid: false, - HTMLErrors: []string{"Parse error"}, - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateHTMLValidityCheck(tt.results) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Content { - t.Errorf("Category = %v, want %v", check.Category, api.Content) - } - }) - } -} - -func TestGenerateLinkChecks(t *testing.T) { - tests := []struct { - name string - results *ContentResults - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "All links valid", - results: &ContentResults{ - Links: []LinkCheck{ - {URL: "https://example.com", Valid: true, Status: 200}, - {URL: "https://example.org", Valid: true, Status: 200}, - }, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 4, - }, - { - name: "Broken links", - results: &ContentResults{ - Links: []LinkCheck{ - {URL: "https://example.com", Valid: true, Status: 404, Error: "Not found"}, - }, - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - { - name: "Links with warnings", - results: &ContentResults{ - Links: []LinkCheck{ - {URL: "https://example.com", Valid: true, Warning: "Could not verify"}, - }, - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 3, - }, - { - name: "No links", - results: &ContentResults{}, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := analyzer.generateLinkChecks(tt.results) - - if tt.name == "No links" { - if len(checks) != 0 { - t.Errorf("Expected no checks, got %d", len(checks)) - } - return - } - - if len(checks) == 0 { - t.Fatal("Expected at least one check") - } - - check := checks[0] - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - }) - } -} - -func TestGenerateImageChecks(t *testing.T) { - tests := []struct { - name string - results *ContentResults - expectedStatus api.CheckStatus - }{ - { - name: "All images have alt", - results: &ContentResults{ - Images: []ImageCheck{ - {Src: "img1.jpg", HasAlt: true, AltText: "Alt 1"}, - {Src: "img2.jpg", HasAlt: true, AltText: "Alt 2"}, - }, - }, - expectedStatus: api.CheckStatusPass, - }, - { - name: "No images have alt", - results: &ContentResults{ - Images: []ImageCheck{ - {Src: "img1.jpg", HasAlt: false}, - {Src: "img2.jpg", HasAlt: false}, - }, - }, - expectedStatus: api.CheckStatusFail, - }, - { - name: "Some images have alt", - results: &ContentResults{ - Images: []ImageCheck{ - {Src: "img1.jpg", HasAlt: true, AltText: "Alt 1"}, - {Src: "img2.jpg", HasAlt: false}, - }, - }, - expectedStatus: api.CheckStatusWarn, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := analyzer.generateImageChecks(tt.results) - - if len(checks) == 0 { - t.Fatal("Expected at least one check") - } - - check := checks[0] - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Category != api.Content { - t.Errorf("Category = %v, want %v", check.Category, api.Content) - } - }) - } -} - -func TestGenerateUnsubscribeCheck(t *testing.T) { - tests := []struct { - name string - results *ContentResults - expectedStatus api.CheckStatus - }{ - { - name: "Has unsubscribe link", - results: &ContentResults{ - HasUnsubscribe: true, - UnsubscribeLinks: []string{"https://example.com/unsubscribe"}, - }, - expectedStatus: api.CheckStatusPass, - }, - { - name: "No unsubscribe link", - results: &ContentResults{}, - expectedStatus: api.CheckStatusWarn, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateUnsubscribeCheck(tt.results) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Category != api.Content { - t.Errorf("Category = %v, want %v", check.Category, api.Content) - } - }) - } -} - -func TestGenerateTextConsistencyCheck(t *testing.T) { - tests := []struct { - name string - results *ContentResults - expectedStatus api.CheckStatus - }{ - { - name: "High consistency", - results: &ContentResults{ - TextPlainRatio: 0.8, - }, - expectedStatus: api.CheckStatusPass, - }, - { - name: "Low consistency", - results: &ContentResults{ - TextPlainRatio: 0.1, - }, - expectedStatus: api.CheckStatusWarn, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateTextConsistencyCheck(tt.results) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - }) - } -} - -func TestGenerateImageRatioCheck(t *testing.T) { - tests := []struct { - name string - results *ContentResults - expectedStatus api.CheckStatus - }{ - { - name: "Reasonable ratio", - results: &ContentResults{ - ImageTextRatio: 3.0, - Images: []ImageCheck{{}, {}, {}}, - }, - expectedStatus: api.CheckStatusPass, - }, - { - name: "High ratio", - results: &ContentResults{ - ImageTextRatio: 7.0, - Images: make([]ImageCheck, 7), - }, - expectedStatus: api.CheckStatusWarn, - }, - { - name: "Excessive ratio", - results: &ContentResults{ - ImageTextRatio: 15.0, - Images: make([]ImageCheck, 15), - }, - expectedStatus: api.CheckStatusFail, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateImageRatioCheck(tt.results) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - }) - } -} - -func TestGenerateSuspiciousURLCheck(t *testing.T) { - results := &ContentResults{ - SuspiciousURLs: []string{ - "https://bit.ly/abc123", - "https://192.168.1.1/page", - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - check := analyzer.generateSuspiciousURLCheck(results) - - if check.Status != api.CheckStatusWarn { - t.Errorf("Status = %v, want %v", check.Status, api.CheckStatusWarn) - } - if check.Category != api.Content { - t.Errorf("Category = %v, want %v", check.Category, api.Content) - } - if !strings.Contains(check.Message, "2") { - t.Error("Message should mention the count of suspicious URLs") - } -} - -func TestGetContentScore(t *testing.T) { - tests := []struct { - name string - results *ContentResults - minScore int - maxScore int - }{ - { - name: "Nil results", - results: nil, - minScore: 0, - maxScore: 0, - }, - { - name: "Perfect content", - results: &ContentResults{ - HTMLValid: true, - Links: []LinkCheck{{Valid: true, Status: 200}}, - Images: []ImageCheck{{HasAlt: true}}, - HasUnsubscribe: true, - TextPlainRatio: 0.8, - ImageTextRatio: 3.0, - }, - minScore: 90, - maxScore: 100, - }, - { - name: "Poor content", - results: &ContentResults{ - HTMLValid: false, - Links: []LinkCheck{{Valid: true, Status: 404}}, - Images: []ImageCheck{{HasAlt: false}}, - HasUnsubscribe: false, - TextPlainRatio: 0.1, - ImageTextRatio: 15.0, - SuspiciousURLs: []string{"url1", "url2"}, - }, - minScore: 0, - maxScore: 25, - }, - { - name: "Average content", - results: &ContentResults{ - HTMLValid: true, - Links: []LinkCheck{{Valid: true, Status: 200}}, - Images: []ImageCheck{{HasAlt: true}}, - HasUnsubscribe: false, - TextPlainRatio: 0.5, - ImageTextRatio: 4.0, - }, - minScore: 50, - maxScore: 90, - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - score := analyzer.GetContentScore(tt.results) - - if score < tt.minScore || score > tt.maxScore { - t.Errorf("GetContentScore() = %v, want between %v and %v", score, tt.minScore, tt.maxScore) - } - - // Ensure score is capped at 100 - if score > 100 { - t.Errorf("Score %v exceeds maximum of 100", score) - } - - // Ensure score is not negative - if score < 0 { - t.Errorf("Score %v is negative", score) - } - }) - } -} - -func TestGenerateContentChecks(t *testing.T) { - tests := []struct { - name string - results *ContentResults - minChecks int - }{ - { - name: "Nil results", - results: nil, - minChecks: 0, - }, - { - name: "Complete results", - results: &ContentResults{ - HTMLValid: true, - Links: []LinkCheck{{Valid: true}}, - Images: []ImageCheck{{HasAlt: true}}, - HasUnsubscribe: true, - TextContent: "Plain text", - HTMLContent: "

HTML text

", - ImageTextRatio: 3.0, - }, - minChecks: 5, // HTML, Links, Images, Unsubscribe, Text consistency, Image ratio - }, - { - name: "With suspicious URLs", - results: &ContentResults{ - HTMLValid: true, - SuspiciousURLs: []string{"url1"}, - }, - minChecks: 3, // HTML, Unsubscribe, Suspicious URLs - }, - } - - analyzer := NewContentAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := analyzer.GenerateContentChecks(tt.results) - - if len(checks) < tt.minChecks { - t.Errorf("Got %d checks, want at least %d", len(checks), tt.minChecks) - } - - // Verify all checks have the Content category - for _, check := range checks { - if check.Category != api.Content { - t.Errorf("Check %s has category %v, want %v", check.Name, check.Category, api.Content) - } - } - }) - } -} - // Helper functions for testing func parseHTML(htmlStr string) (*html.Node, error) { diff --git a/pkg/analyzer/dns.go b/pkg/analyzer/dns.go index 1a03a99..0f7c111 100644 --- a/pkg/analyzer/dns.go +++ b/pkg/analyzer/dns.go @@ -492,232 +492,3 @@ func (d *DNSAnalyzer) validateBIMI(record string) bool { return true } - -// GenerateDNSChecks generates check results for DNS validation -func (d *DNSAnalyzer) GenerateDNSChecks(results *DNSResults) []api.Check { - var checks []api.Check - - if results == nil { - return checks - } - - // MX record check - checks = append(checks, d.generateMXCheck(results)) - - // SPF record check - if results.SPFRecord != nil { - checks = append(checks, d.generateSPFCheck(results.SPFRecord)) - } - - // DKIM record checks - for _, dkim := range results.DKIMRecords { - checks = append(checks, d.generateDKIMCheck(&dkim)) - } - - // DMARC record check - if results.DMARCRecord != nil { - checks = append(checks, d.generateDMARCCheck(results.DMARCRecord)) - } - - // BIMI record check (optional) - if results.BIMIRecord != nil { - checks = append(checks, d.generateBIMICheck(results.BIMIRecord, results.DMARCRecord)) - } - - return checks -} - -// generateMXCheck creates a check for MX records -func (d *DNSAnalyzer) generateMXCheck(results *DNSResults) api.Check { - check := api.Check{ - Category: api.Dns, - Name: "MX Records", - } - - if len(results.MXRecords) == 0 || !results.MXRecords[0].Valid { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Severity = api.PtrTo(api.CheckSeverityCritical) - - if len(results.MXRecords) > 0 && results.MXRecords[0].Error != "" { - check.Message = results.MXRecords[0].Error - } else { - check.Message = "No valid MX records found" - } - check.Advice = api.PtrTo("Configure MX records for your domain to receive email") - } else { - check.Status = api.CheckStatusPass - check.Score = 100 - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = fmt.Sprintf("Found %d valid MX record(s)", len(results.MXRecords)) - - // Add details about MX records - var mxList []string - for _, mx := range results.MXRecords { - mxList = append(mxList, fmt.Sprintf("%s (priority %d)", mx.Host, mx.Priority)) - } - details := strings.Join(mxList, ", ") - check.Details = &details - check.Advice = api.PtrTo("Your MX records are properly configured") - } - - return check -} - -// generateSPFCheck creates a check for SPF records -func (d *DNSAnalyzer) generateSPFCheck(spf *SPFRecord) api.Check { - check := api.Check{ - Category: api.Dns, - Name: "SPF Record", - } - - if !spf.Valid { - if spf.Record == "" { - // If no record exists at all, it's a failure - check.Status = api.CheckStatusFail - check.Score = 0 - check.Message = spf.Error - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("Configure an SPF record for your domain to improve deliverability") - } else { - // If record exists but is invalid, it's a failure - check.Status = api.CheckStatusFail - check.Score = 5 - check.Message = "SPF record found but appears invalid" - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Review and fix your SPF record syntax") - check.Details = &spf.Record - } - } else { - check.Status = api.CheckStatusPass - check.Score = 100 - check.Message = "Valid SPF record found" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Details = &spf.Record - check.Advice = api.PtrTo("Your SPF record is properly configured") - } - - return check -} - -// generateDKIMCheck creates a check for DKIM records -func (d *DNSAnalyzer) generateDKIMCheck(dkim *DKIMRecord) api.Check { - check := api.Check{ - Category: api.Dns, - Name: fmt.Sprintf("DKIM Record (%s)", dkim.Selector), - } - - if !dkim.Valid { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Message = fmt.Sprintf("DKIM record not found or invalid: %s", dkim.Error) - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Ensure DKIM record is published in DNS for the selector used") - details := fmt.Sprintf("Selector: %s, Domain: %s", dkim.Selector, dkim.Domain) - check.Details = &details - } else { - check.Status = api.CheckStatusPass - check.Score = 100 - check.Message = "Valid DKIM record found" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - details := fmt.Sprintf("Selector: %s, Domain: %s", dkim.Selector, dkim.Domain) - check.Details = &details - check.Advice = api.PtrTo("Your DKIM record is properly published") - } - - return check -} - -// generateDMARCCheck creates a check for DMARC records -func (d *DNSAnalyzer) generateDMARCCheck(dmarc *DMARCRecord) api.Check { - check := api.Check{ - Category: api.Dns, - Name: "DMARC Record", - } - - if !dmarc.Valid { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Message = dmarc.Error - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Configure a DMARC record for your domain to improve deliverability and prevent spoofing") - } else { - check.Status = api.CheckStatusPass - check.Score = 100 - check.Message = fmt.Sprintf("Valid DMARC record found with policy: %s", dmarc.Policy) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Details = &dmarc.Record - - // Provide advice based on policy - switch dmarc.Policy { - case "none": - advice := "DMARC policy is set to 'none' (monitoring only). Consider upgrading to 'quarantine' or 'reject' for better protection" - check.Advice = &advice - case "quarantine": - advice := "DMARC policy is set to 'quarantine'. This provides good protection" - check.Advice = &advice - case "reject": - advice := "DMARC policy is set to 'reject'. This provides the strongest protection" - check.Advice = &advice - default: - advice := "Your DMARC record is properly configured" - check.Advice = &advice - } - } - - return check -} - -// generateBIMICheck creates a check for BIMI records -func (d *DNSAnalyzer) generateBIMICheck(bimi *BIMIRecord, dmarc *DMARCRecord) api.Check { - check := api.Check{ - Category: api.Dns, - Name: "BIMI Record", - } - - if !bimi.Valid { - // BIMI is optional, so missing record is just informational - if bimi.Record == "" { - check.Status = api.CheckStatusInfo - check.Score = 0 - check.Message = "No BIMI record found (optional)" - check.Severity = api.PtrTo(api.CheckSeverityLow) - if dmarc.Policy != "quarantine" && dmarc.Policy != "reject" { - check.Advice = api.PtrTo("BIMI is optional. Consider implementing it to display your brand logo in supported email clients. Requires enforced DMARC policy (p=quarantine or p=reject)") - } else { - check.Advice = api.PtrTo("BIMI is optional. Consider implementing it to display your brand logo in supported email clients.") - } - } else { - // If record exists but is invalid - check.Status = api.CheckStatusWarn - check.Score = 5 - check.Message = fmt.Sprintf("BIMI record found but invalid: %s", bimi.Error) - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Advice = api.PtrTo("Review and fix your BIMI record syntax. Ensure it contains v=BIMI1 and a valid logo URL (l=)") - check.Details = &bimi.Record - } - } else { - check.Status = api.CheckStatusPass - check.Score = 100 // BIMI doesn't contribute to score (branding feature) - check.Message = "Valid BIMI record found" - check.Severity = api.PtrTo(api.CheckSeverityInfo) - - // Build details with logo and VMC URLs - var detailsParts []string - detailsParts = append(detailsParts, fmt.Sprintf("Selector: %s", bimi.Selector)) - if bimi.LogoURL != "" { - detailsParts = append(detailsParts, fmt.Sprintf("Logo URL: %s", bimi.LogoURL)) - } - if bimi.VMCURL != "" { - detailsParts = append(detailsParts, fmt.Sprintf("VMC URL: %s", bimi.VMCURL)) - check.Advice = api.PtrTo("Your BIMI record is properly configured with a Verified Mark Certificate") - } else { - check.Advice = api.PtrTo("Your BIMI record is properly configured. Consider adding a Verified Mark Certificate (VMC) for enhanced trust") - } - - details := strings.Join(detailsParts, ", ") - check.Details = &details - } - - return check -} diff --git a/pkg/analyzer/dns_test.go b/pkg/analyzer/dns_test.go index 750c620..7859523 100644 --- a/pkg/analyzer/dns_test.go +++ b/pkg/analyzer/dns_test.go @@ -23,11 +23,8 @@ package analyzer import ( "net/mail" - "strings" "testing" "time" - - "git.happydns.org/happyDeliver/internal/api" ) func TestNewDNSAnalyzer(t *testing.T) { @@ -300,338 +297,6 @@ func TestValidateDMARC(t *testing.T) { } } -func TestGenerateMXCheck(t *testing.T) { - tests := []struct { - name string - results *DNSResults - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Valid MX records", - results: &DNSResults{ - Domain: "example.com", - MXRecords: []MXRecord{ - {Host: "mail.example.com", Priority: 10, Valid: true}, - {Host: "mail2.example.com", Priority: 20, Valid: true}, - }, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "No MX records", - results: &DNSResults{ - Domain: "example.com", - MXRecords: []MXRecord{ - {Valid: false, Error: "No MX records found"}, - }, - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - { - name: "MX lookup failed", - results: &DNSResults{ - Domain: "example.com", - MXRecords: []MXRecord{ - {Valid: false, Error: "DNS lookup failed"}, - }, - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewDNSAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateMXCheck(tt.results) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Dns { - t.Errorf("Category = %v, want %v", check.Category, api.Dns) - } - }) - } -} - -func TestGenerateSPFCheck(t *testing.T) { - tests := []struct { - name string - spf *SPFRecord - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Valid SPF", - spf: &SPFRecord{ - Record: "v=spf1 include:_spf.example.com -all", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "Invalid SPF", - spf: &SPFRecord{ - Record: "v=spf1 invalid syntax", - Valid: false, - Error: "SPF record appears malformed", - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 5, - }, - { - name: "No SPF record", - spf: &SPFRecord{ - Valid: false, - Error: "No SPF record found", - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewDNSAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateSPFCheck(tt.spf) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Dns { - t.Errorf("Category = %v, want %v", check.Category, api.Dns) - } - }) - } -} - -func TestGenerateDKIMCheck(t *testing.T) { - tests := []struct { - name string - dkim *DKIMRecord - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Valid DKIM", - dkim: &DKIMRecord{ - Selector: "default", - Domain: "example.com", - Record: "v=DKIM1; k=rsa; p=MIGfMA0...", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "Invalid DKIM", - dkim: &DKIMRecord{ - Selector: "default", - Domain: "example.com", - Valid: false, - Error: "No DKIM record found", - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewDNSAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateDKIMCheck(tt.dkim) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Dns { - t.Errorf("Category = %v, want %v", check.Category, api.Dns) - } - if !strings.Contains(check.Name, tt.dkim.Selector) { - t.Errorf("Check name should contain selector %s", tt.dkim.Selector) - } - }) - } -} - -func TestGenerateDMARCCheck(t *testing.T) { - tests := []struct { - name string - dmarc *DMARCRecord - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Valid DMARC - reject", - dmarc: &DMARCRecord{ - Record: "v=DMARC1; p=reject", - Policy: "reject", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "Valid DMARC - quarantine", - dmarc: &DMARCRecord{ - Record: "v=DMARC1; p=quarantine", - Policy: "quarantine", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "Valid DMARC - none", - dmarc: &DMARCRecord{ - Record: "v=DMARC1; p=none", - Policy: "none", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 10, - }, - { - name: "No DMARC record", - dmarc: &DMARCRecord{ - Valid: false, - Error: "No DMARC record found", - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewDNSAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateDMARCCheck(tt.dmarc) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Dns { - t.Errorf("Category = %v, want %v", check.Category, api.Dns) - } - - // Check that advice mentions policy for valid DMARC - if tt.dmarc.Valid && check.Advice != nil { - if tt.dmarc.Policy == "none" && !strings.Contains(*check.Advice, "none") { - t.Error("Advice should mention 'none' policy") - } - } - }) - } -} - -func TestGenerateDNSChecks(t *testing.T) { - tests := []struct { - name string - results *DNSResults - minChecks int - }{ - { - name: "Nil results", - results: nil, - minChecks: 0, - }, - { - name: "Complete results", - results: &DNSResults{ - Domain: "example.com", - MXRecords: []MXRecord{ - {Host: "mail.example.com", Priority: 10, Valid: true}, - }, - SPFRecord: &SPFRecord{ - Record: "v=spf1 include:_spf.example.com -all", - Valid: true, - }, - DKIMRecords: []DKIMRecord{ - { - Selector: "default", - Domain: "example.com", - Valid: true, - }, - }, - DMARCRecord: &DMARCRecord{ - Record: "v=DMARC1; p=quarantine", - Policy: "quarantine", - Valid: true, - }, - }, - minChecks: 4, // MX, SPF, DKIM, DMARC - }, - { - name: "Partial results", - results: &DNSResults{ - Domain: "example.com", - MXRecords: []MXRecord{ - {Host: "mail.example.com", Priority: 10, Valid: true}, - }, - }, - minChecks: 1, // Only MX - }, - } - - analyzer := NewDNSAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := analyzer.GenerateDNSChecks(tt.results) - - if len(checks) < tt.minChecks { - t.Errorf("Got %d checks, want at least %d", len(checks), tt.minChecks) - } - - // Verify all checks have the DNS category - for _, check := range checks { - if check.Category != api.Dns { - t.Errorf("Check %s has category %v, want %v", check.Name, check.Category, api.Dns) - } - } - }) - } -} - -func TestAnalyzeDNS_NoDomain(t *testing.T) { - analyzer := NewDNSAnalyzer(5 * time.Second) - email := &EmailMessage{ - Header: make(mail.Header), - // No From address - } - - results := analyzer.AnalyzeDNS(email, nil) - - if results == nil { - t.Fatal("Expected results, got nil") - } - - if len(results.Errors) == 0 { - t.Error("Expected error when no domain can be extracted") - } -} - func TestExtractBIMITag(t *testing.T) { tests := []struct { name string @@ -732,89 +397,3 @@ func TestValidateBIMI(t *testing.T) { }) } } - -func TestGenerateBIMICheck(t *testing.T) { - tests := []struct { - name string - bimi *BIMIRecord - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Valid BIMI with logo only", - bimi: &BIMIRecord{ - Selector: "default", - Domain: "example.com", - Record: "v=BIMI1; l=https://example.com/logo.svg", - LogoURL: "https://example.com/logo.svg", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 0, // BIMI doesn't contribute to score - }, - { - name: "Valid BIMI with VMC", - bimi: &BIMIRecord{ - Selector: "default", - Domain: "example.com", - Record: "v=BIMI1; l=https://example.com/logo.svg; a=https://example.com/vmc.pem", - LogoURL: "https://example.com/logo.svg", - VMCURL: "https://example.com/vmc.pem", - Valid: true, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 0, - }, - { - name: "No BIMI record (optional)", - bimi: &BIMIRecord{ - Selector: "default", - Domain: "example.com", - Valid: false, - Error: "No BIMI record found", - }, - expectedStatus: api.CheckStatusInfo, - expectedScore: 0, - }, - { - name: "Invalid BIMI record", - bimi: &BIMIRecord{ - Selector: "default", - Domain: "example.com", - Record: "v=BIMI1", - Valid: false, - Error: "BIMI record appears malformed", - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 0, - }, - } - - analyzer := NewDNSAnalyzer(5 * time.Second) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateBIMICheck(tt.bimi) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Dns { - t.Errorf("Category = %v, want %v", check.Category, api.Dns) - } - if check.Name != "BIMI Record" { - t.Errorf("Name = %q, want %q", check.Name, "BIMI Record") - } - - // Check details for valid BIMI with VMC - if tt.bimi.Valid && tt.bimi.VMCURL != "" && check.Details != nil { - if !strings.Contains(*check.Details, "VMC URL") { - t.Error("Details should contain VMC URL for valid BIMI with VMC") - } - } - }) - } -} diff --git a/pkg/analyzer/headers.go b/pkg/analyzer/headers.go index 7fa252a..4ffc1a3 100644 --- a/pkg/analyzer/headers.go +++ b/pkg/analyzer/headers.go @@ -36,56 +36,53 @@ func NewHeaderAnalyzer() *HeaderAnalyzer { return &HeaderAnalyzer{} } -// calculateHeaderScore evaluates email structural quality -func (h *HeaderAnalyzer) calculateHeaderScore(email *EmailMessage) int { - if email == nil { +// CalculateHeaderScore evaluates email structural quality from header analysis +func (h *HeaderAnalyzer) CalculateHeaderScore(analysis *api.HeaderAnalysis) int { + if analysis == nil || analysis.Headers == nil { return 0 } score := 0 - requiredHeaders := 0 - presentHeaders := 0 + headers := *analysis.Headers - // Check required headers (RFC 5322) - headers := map[string]bool{ - "From": false, - "Date": false, - "Message-ID": false, - } + // Check required headers (RFC 5322) - 40 points + requiredHeaders := []string{"from", "date", "message-id"} + requiredCount := len(requiredHeaders) + presentRequired := 0 - for header := range headers { - requiredHeaders++ - if email.HasHeader(header) && email.GetHeaderValue(header) != "" { - headers[header] = true - presentHeaders++ + for _, headerName := range requiredHeaders { + if check, exists := headers[headerName]; exists && check.Present { + presentRequired++ } } - // Score based on required headers (40 points) - if presentHeaders == requiredHeaders { + if presentRequired == requiredCount { score += 40 } else { - score += int(40 * (float32(presentHeaders) / float32(requiredHeaders))) + score += int(40 * (float32(presentRequired) / float32(requiredCount))) } // Check recommended headers (30 points) - recommendedHeaders := []string{"Subject", "To", "Reply-To"} - recommendedPresent := 0 - for _, header := range recommendedHeaders { - if email.HasHeader(header) && email.GetHeaderValue(header) != "" { - recommendedPresent++ + recommendedHeaders := []string{"subject", "to", "reply-to"} + recommendedCount := len(recommendedHeaders) + presentRecommended := 0 + + for _, headerName := range recommendedHeaders { + if check, exists := headers[headerName]; exists && check.Present { + presentRecommended++ } } - score += int(30 * (float32(recommendedPresent) / float32(len(recommendedHeaders)))) + score += int(30 * (float32(presentRecommended) / float32(recommendedCount))) // Check for proper MIME structure (20 points) - if len(email.Parts) > 0 { + if analysis.HasMimeStructure != nil && *analysis.HasMimeStructure { score += 20 } - // Check Message-ID format (10 point) - if messageID := email.GetHeaderValue("Message-ID"); messageID != "" { - if h.isValidMessageID(messageID) { + // Check Message-ID format (10 points) + if check, exists := headers["message-id"]; exists && check.Present { + // If Valid is set and true, award points + if check.Valid != nil && *check.Valid { score += 10 } } @@ -123,181 +120,187 @@ func (h *HeaderAnalyzer) isValidMessageID(messageID string) bool { return len(parts[0]) > 0 && len(parts[1]) > 0 } -// GenerateHeaderChecks creates checks for email header quality -func (h *HeaderAnalyzer) GenerateHeaderChecks(email *EmailMessage) []api.Check { - var checks []api.Check - +// GenerateHeaderAnalysis creates structured header analysis from email +func (h *HeaderAnalyzer) GenerateHeaderAnalysis(email *EmailMessage) *api.HeaderAnalysis { if email == nil { - return checks + return nil } - // Required headers check - checks = append(checks, h.generateRequiredHeadersCheck(email)) + analysis := &api.HeaderAnalysis{} - // Recommended headers check - checks = append(checks, h.generateRecommendedHeadersCheck(email)) + // Check for proper MIME structure + analysis.HasMimeStructure = api.PtrTo(len(email.Parts) > 0) - // Message-ID check - checks = append(checks, h.generateMessageIDCheck(email)) + // Initialize headers map + headers := make(map[string]api.HeaderCheck) - // MIME structure check - checks = append(checks, h.generateMIMEStructureCheck(email)) + // Check required headers + requiredHeaders := []string{"From", "To", "Date", "Message-ID", "Subject"} + for _, headerName := range requiredHeaders { + check := h.checkHeader(email, headerName, "required") + headers[strings.ToLower(headerName)] = *check + } - return checks + // Check recommended headers + recommendedHeaders := []string{"Reply-To", "Return-Path"} + for _, headerName := range recommendedHeaders { + check := h.checkHeader(email, headerName, "recommended") + headers[strings.ToLower(headerName)] = *check + } + + // Check optional headers + optionalHeaders := []string{"List-Unsubscribe", "List-Unsubscribe-Post", "Precedence"} + for _, headerName := range optionalHeaders { + check := h.checkHeader(email, headerName, "optional") + headers[strings.ToLower(headerName)] = *check + } + + analysis.Headers = &headers + + // Domain alignment + domainAlignment := h.analyzeDomainAlignment(email) + if domainAlignment != nil { + analysis.DomainAlignment = domainAlignment + } + + // Header issues + issues := h.findHeaderIssues(email) + if len(issues) > 0 { + analysis.Issues = &issues + } + + return analysis } -// generateRequiredHeadersCheck checks for required RFC 5322 headers -func (h *HeaderAnalyzer) generateRequiredHeadersCheck(email *EmailMessage) api.Check { - check := api.Check{ - Category: api.Headers, - Name: "Required Headers", +// checkHeader checks if a header is present and valid +func (h *HeaderAnalyzer) checkHeader(email *EmailMessage, headerName string, importance string) *api.HeaderCheck { + value := email.GetHeaderValue(headerName) + present := email.HasHeader(headerName) && value != "" + + importanceEnum := api.HeaderCheckImportance(importance) + check := &api.HeaderCheck{ + Present: present, + Importance: &importanceEnum, } - requiredHeaders := []string{"From", "Date", "Message-ID"} - missing := []string{} + if present { + check.Value = &value + // Validate specific headers + valid := true + var headerIssues []string + + switch headerName { + case "Message-ID": + if !h.isValidMessageID(value) { + valid = false + headerIssues = append(headerIssues, "Invalid Message-ID format (should be )") + } + case "Date": + // Could add date validation here + } + + check.Valid = &valid + if len(headerIssues) > 0 { + check.Issues = &headerIssues + } + } else { + valid := false + check.Valid = &valid + if importance == "required" { + issues := []string{"Required header is missing"} + check.Issues = &issues + } + } + + return check +} + +// analyzeDomainAlignment checks domain alignment between headers +func (h *HeaderAnalyzer) analyzeDomainAlignment(email *EmailMessage) *api.DomainAlignment { + alignment := &api.DomainAlignment{ + Aligned: api.PtrTo(true), + } + + // Extract From domain + fromAddr := email.GetHeaderValue("From") + if fromAddr != "" { + domain := h.extractDomain(fromAddr) + if domain != "" { + alignment.FromDomain = &domain + } + } + + // Extract Return-Path domain + returnPath := email.GetHeaderValue("Return-Path") + if returnPath != "" { + domain := h.extractDomain(returnPath) + if domain != "" { + alignment.ReturnPathDomain = &domain + } + } + + // Check alignment + issues := []string{} + if alignment.FromDomain != nil && alignment.ReturnPathDomain != nil { + if *alignment.FromDomain != *alignment.ReturnPathDomain { + *alignment.Aligned = false + issues = append(issues, "Return-Path domain does not match From domain") + } + } + + if len(issues) > 0 { + alignment.Issues = &issues + } + + return alignment +} + +// extractDomain extracts domain from email address +func (h *HeaderAnalyzer) extractDomain(emailAddr string) string { + // Remove angle brackets if present + emailAddr = strings.Trim(emailAddr, "<> ") + + // Find @ symbol + atIndex := strings.LastIndex(emailAddr, "@") + if atIndex == -1 { + return "" + } + + domain := emailAddr[atIndex+1:] + // Remove any trailing > + domain = strings.TrimRight(domain, ">") + + return domain +} + +// findHeaderIssues identifies issues with headers +func (h *HeaderAnalyzer) findHeaderIssues(email *EmailMessage) []api.HeaderIssue { + var issues []api.HeaderIssue + + // Check for missing required headers + requiredHeaders := []string{"From", "Date", "Message-ID"} for _, header := range requiredHeaders { if !email.HasHeader(header) || email.GetHeaderValue(header) == "" { - missing = append(missing, header) + issues = append(issues, api.HeaderIssue{ + Header: header, + Severity: api.HeaderIssueSeverityCritical, + Message: fmt.Sprintf("Required header '%s' is missing", header), + Advice: api.PtrTo(fmt.Sprintf("Add the %s header to ensure RFC 5322 compliance", header)), + }) } } - if len(missing) == 0 { - check.Status = api.CheckStatusPass - check.Score = 4.0 - check.Grade = ScoreToCheckGrade((4.0 / 10.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "All required headers are present" - check.Advice = api.PtrTo("Your email has proper RFC 5322 headers") - } else { - check.Status = api.CheckStatusFail - check.Score = 0.0 - check.Grade = ScoreToCheckGrade(0.0) - check.Severity = api.PtrTo(api.CheckSeverityCritical) - check.Message = fmt.Sprintf("Missing required header(s): %s", strings.Join(missing, ", ")) - check.Advice = api.PtrTo("Add all required headers to ensure email deliverability") - details := fmt.Sprintf("Missing: %s", strings.Join(missing, ", ")) - check.Details = &details - } - - return check -} - -// generateRecommendedHeadersCheck checks for recommended headers -func (h *HeaderAnalyzer) generateRecommendedHeadersCheck(email *EmailMessage) api.Check { - check := api.Check{ - Category: api.Headers, - Name: "Recommended Headers", - } - - recommendedHeaders := []string{"Subject", "To", "Reply-To"} - missing := []string{} - - for _, header := range recommendedHeaders { - if !email.HasHeader(header) || email.GetHeaderValue(header) == "" { - missing = append(missing, header) - } - } - - if len(missing) == 0 { - check.Status = api.CheckStatusPass - check.Score = 30 - check.Grade = ScoreToCheckGrade((3.0 / 10.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "All recommended headers are present" - check.Advice = api.PtrTo("Your email includes all recommended headers") - } else if len(missing) < len(recommendedHeaders) { - check.Status = api.CheckStatusWarn - check.Score = 15 - check.Grade = ScoreToCheckGrade((1.5 / 10.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = fmt.Sprintf("Missing some recommended header(s): %s", strings.Join(missing, ", ")) - check.Advice = api.PtrTo("Consider adding recommended headers for better deliverability") - details := fmt.Sprintf("Missing: %s", strings.Join(missing, ", ")) - check.Details = &details - } else { - check.Status = api.CheckStatusWarn - check.Score = 0 - check.Grade = ScoreToCheckGrade(0.0) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Message = "Missing all recommended headers" - check.Advice = api.PtrTo("Add recommended headers (Subject, To, Reply-To) for better email presentation") - } - - return check -} - -// generateMessageIDCheck validates Message-ID header -func (h *HeaderAnalyzer) generateMessageIDCheck(email *EmailMessage) api.Check { - check := api.Check{ - Category: api.Headers, - Name: "Message-ID Format", - } - + // Check Message-ID format messageID := email.GetHeaderValue("Message-ID") - - if messageID == "" { - check.Status = api.CheckStatusFail - check.Score = 0 - check.Grade = ScoreToCheckGrade(0.0) - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Message = "Message-ID header is missing" - check.Advice = api.PtrTo("Add a unique Message-ID header to your email") - } else if !h.isValidMessageID(messageID) { - check.Status = api.CheckStatusWarn - check.Score = 5 - check.Grade = ScoreToCheckGrade((0.5 / 10.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Message = "Message-ID format is invalid" - check.Advice = api.PtrTo("Use proper Message-ID format: ") - check.Details = &messageID - } else { - check.Status = api.CheckStatusPass - check.Score = 10 - check.Grade = ScoreToCheckGrade((1.0 / 10.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = "Message-ID is properly formatted" - check.Advice = api.PtrTo("Your Message-ID follows RFC 5322 standards") - check.Details = &messageID + if messageID != "" && !h.isValidMessageID(messageID) { + issues = append(issues, api.HeaderIssue{ + Header: "Message-ID", + Severity: api.HeaderIssueSeverityMedium, + Message: "Message-ID format is invalid", + Advice: api.PtrTo("Use proper Message-ID format: "), + }) } - return check -} - -// generateMIMEStructureCheck validates MIME structure -func (h *HeaderAnalyzer) generateMIMEStructureCheck(email *EmailMessage) api.Check { - check := api.Check{ - Category: api.Headers, - Name: "MIME Structure", - } - - if len(email.Parts) == 0 { - check.Status = api.CheckStatusWarn - check.Score = 0.0 - check.Grade = ScoreToCheckGrade(0.0) - check.Severity = api.PtrTo(api.CheckSeverityLow) - check.Message = "No MIME parts detected" - check.Advice = api.PtrTo("Consider using multipart MIME for better compatibility") - } else { - check.Status = api.CheckStatusPass - check.Score = 2.0 - check.Grade = ScoreToCheckGrade((2.0 / 10.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = fmt.Sprintf("Proper MIME structure with %d part(s)", len(email.Parts)) - check.Advice = api.PtrTo("Your email has proper MIME structure") - - // Add details about parts - partTypes := []string{} - for _, part := range email.Parts { - if part.ContentType != "" { - partTypes = append(partTypes, part.ContentType) - } - } - if len(partTypes) > 0 { - details := fmt.Sprintf("Parts: %s", strings.Join(partTypes, ", ")) - check.Details = &details - } - } - - return check + return issues } diff --git a/pkg/analyzer/headers_test.go b/pkg/analyzer/headers_test.go index 8594f7f..418b553 100644 --- a/pkg/analyzer/headers_test.go +++ b/pkg/analyzer/headers_test.go @@ -25,8 +25,6 @@ import ( "net/mail" "net/textproto" "testing" - - "git.happydns.org/happyDeliver/internal/api" ) func TestCalculateHeaderScore(t *testing.T) { @@ -109,95 +107,70 @@ func TestCalculateHeaderScore(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - score := analyzer.calculateHeaderScore(tt.email) + // Generate header analysis first + analysis := analyzer.GenerateHeaderAnalysis(tt.email) + score := analyzer.CalculateHeaderScore(analysis) if score < tt.minScore || score > tt.maxScore { - t.Errorf("calculateHeaderScore() = %v, want between %v and %v", score, tt.minScore, tt.maxScore) + t.Errorf("CalculateHeaderScore() = %v, want between %v and %v", score, tt.minScore, tt.maxScore) } }) } } -func TestGenerateRequiredHeadersCheck(t *testing.T) { +func TestCheckHeader(t *testing.T) { tests := []struct { - name string - email *EmailMessage - expectedStatus api.CheckStatus - expectedScore int + name string + headerName string + headerValue string + importance string + expectedPresent bool + expectedValid bool + expectedIssuesLen int }{ { - name: "All required headers present", - email: &EmailMessage{ - Header: createHeaderWithFields(map[string]string{ - "From": "sender@example.com", - "Date": "Mon, 01 Jan 2024 12:00:00 +0000", - "Message-ID": "", - }), - From: &mail.Address{Address: "sender@example.com"}, - MessageID: "", - Date: "Mon, 01 Jan 2024 12:00:00 +0000", - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 40, + name: "Valid Message-ID", + headerName: "Message-ID", + headerValue: "", + importance: "required", + expectedPresent: true, + expectedValid: true, + expectedIssuesLen: 0, }, { - name: "Missing all required headers", - email: &EmailMessage{ - Header: make(mail.Header), - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, + name: "Invalid Message-ID format", + headerName: "Message-ID", + headerValue: "invalid-message-id", + importance: "required", + expectedPresent: true, + expectedValid: false, + expectedIssuesLen: 1, }, { - name: "Missing some required headers", - email: &EmailMessage{ - Header: createHeaderWithFields(map[string]string{ - "From": "sender@example.com", - }), - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - analyzer := NewHeaderAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateRequiredHeadersCheck(tt.email) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Headers { - t.Errorf("Category = %v, want %v", check.Category, api.Headers) - } - }) - } -} - -func TestGenerateMessageIDCheck(t *testing.T) { - tests := []struct { - name string - messageID string - expectedStatus api.CheckStatus - }{ - { - name: "Valid Message-ID", - messageID: "", - expectedStatus: api.CheckStatusPass, + name: "Missing required header", + headerName: "From", + headerValue: "", + importance: "required", + expectedPresent: false, + expectedValid: false, + expectedIssuesLen: 1, }, { - name: "Invalid Message-ID format", - messageID: "invalid-message-id", - expectedStatus: api.CheckStatusWarn, + name: "Missing optional header", + headerName: "Reply-To", + headerValue: "", + importance: "optional", + expectedPresent: false, + expectedValid: false, + expectedIssuesLen: 0, }, { - name: "Missing Message-ID", - messageID: "", - expectedStatus: api.CheckStatusFail, + name: "Valid Date header", + headerName: "Date", + headerValue: "Mon, 01 Jan 2024 12:00:00 +0000", + importance: "required", + expectedPresent: true, + expectedValid: true, + expectedIssuesLen: 0, }, } @@ -207,86 +180,77 @@ func TestGenerateMessageIDCheck(t *testing.T) { t.Run(tt.name, func(t *testing.T) { email := &EmailMessage{ Header: createHeaderWithFields(map[string]string{ - "Message-ID": tt.messageID, + tt.headerName: tt.headerValue, }), } - check := analyzer.generateMessageIDCheck(email) + check := analyzer.checkHeader(email, tt.headerName, tt.importance) - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) + if check.Present != tt.expectedPresent { + t.Errorf("Present = %v, want %v", check.Present, tt.expectedPresent) } - if check.Category != api.Headers { - t.Errorf("Category = %v, want %v", check.Category, api.Headers) + + if check.Valid != nil && *check.Valid != tt.expectedValid { + t.Errorf("Valid = %v, want %v", *check.Valid, tt.expectedValid) + } + + if check.Importance == nil { + t.Error("Importance is nil") + } else if string(*check.Importance) != tt.importance { + t.Errorf("Importance = %v, want %v", *check.Importance, tt.importance) + } + + issuesLen := 0 + if check.Issues != nil { + issuesLen = len(*check.Issues) + } + if issuesLen != tt.expectedIssuesLen { + t.Errorf("Issues length = %d, want %d", issuesLen, tt.expectedIssuesLen) } }) } } -func TestGenerateMIMEStructureCheck(t *testing.T) { - tests := []struct { - name string - parts []MessagePart - expectedStatus api.CheckStatus - }{ - { - name: "With MIME parts", - parts: []MessagePart{ - {ContentType: "text/plain", Content: "test"}, - {ContentType: "text/html", Content: "

test

"}, - }, - expectedStatus: api.CheckStatusPass, - }, - { - name: "No MIME parts", - parts: []MessagePart{}, - expectedStatus: api.CheckStatusWarn, - }, - } - - analyzer := NewHeaderAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - email := &EmailMessage{ - Header: make(mail.Header), - Parts: tt.parts, - } - - check := analyzer.generateMIMEStructureCheck(email) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - }) - } -} - -func TestGenerateHeaderChecks(t *testing.T) { +func TestHeaderAnalyzer_IsValidMessageID(t *testing.T) { tests := []struct { name string - email *EmailMessage - minChecks int + messageID string + expected bool }{ { - name: "Nil email", - email: nil, - minChecks: 0, + name: "Valid Message-ID", + messageID: "", + expected: true, }, { - name: "Complete email", - email: &EmailMessage{ - Header: createHeaderWithFields(map[string]string{ - "From": "sender@example.com", - "To": "recipient@example.com", - "Subject": "Test", - "Date": "Mon, 01 Jan 2024 12:00:00 +0000", - "Message-ID": "", - "Reply-To": "reply@example.com", - }), - Parts: []MessagePart{{ContentType: "text/plain", Content: "test"}}, - }, - minChecks: 4, // Required, Recommended, Message-ID, MIME + name: "Valid with complex local part", + messageID: "", + expected: true, + }, + { + name: "Missing angle brackets", + messageID: "abc123@example.com", + expected: false, + }, + { + name: "Missing @ symbol", + messageID: "", + expected: false, + }, + { + name: "Empty local part", + messageID: "<@example.com>", + expected: false, + }, + { + name: "Empty domain", + messageID: "", + expected: false, + }, + { + name: "Multiple @ symbols", + messageID: "", + expected: false, }, } @@ -294,17 +258,126 @@ func TestGenerateHeaderChecks(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - checks := analyzer.GenerateHeaderChecks(tt.email) + result := analyzer.isValidMessageID(tt.messageID) + if result != tt.expected { + t.Errorf("isValidMessageID(%q) = %v, want %v", tt.messageID, result, tt.expected) + } + }) + } +} - if len(checks) < tt.minChecks { - t.Errorf("Got %d checks, want at least %d", len(checks), tt.minChecks) +func TestHeaderAnalyzer_ExtractDomain(t *testing.T) { + tests := []struct { + name string + email string + expected string + }{ + { + name: "Simple email", + email: "user@example.com", + expected: "example.com", + }, + { + name: "Email with angle brackets", + email: "", + expected: "example.com", + }, + { + name: "Email with display name", + email: "User Name ", + expected: "example.com", + }, + { + name: "Email with spaces", + email: " user@example.com ", + expected: "example.com", + }, + { + name: "Invalid email", + email: "not-an-email", + expected: "", + }, + { + name: "Empty string", + email: "", + expected: "", + }, + } + + analyzer := NewHeaderAnalyzer() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := analyzer.extractDomain(tt.email) + if result != tt.expected { + t.Errorf("extractDomain(%q) = %q, want %q", tt.email, result, tt.expected) + } + }) + } +} + +func TestAnalyzeDomainAlignment(t *testing.T) { + tests := []struct { + name string + fromHeader string + returnPath string + expectAligned bool + expectIssuesLen int + }{ + { + name: "Aligned domains", + fromHeader: "sender@example.com", + returnPath: "bounce@example.com", + expectAligned: true, + expectIssuesLen: 0, + }, + { + name: "Misaligned domains", + fromHeader: "sender@example.com", + returnPath: "bounce@different.com", + expectAligned: false, + expectIssuesLen: 1, + }, + { + name: "Only From header", + fromHeader: "sender@example.com", + returnPath: "", + expectAligned: true, + expectIssuesLen: 0, + }, + } + + analyzer := NewHeaderAnalyzer() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + email := &EmailMessage{ + Header: createHeaderWithFields(map[string]string{ + "From": tt.fromHeader, + "Return-Path": tt.returnPath, + }), } - // Verify all checks have the Headers category - for _, check := range checks { - if check.Category != api.Headers { - t.Errorf("Check %s has category %v, want %v", check.Name, check.Category, api.Headers) - } + alignment := analyzer.analyzeDomainAlignment(email) + + if alignment == nil { + t.Fatal("Expected non-nil alignment") + } + + if alignment.Aligned == nil { + t.Fatal("Expected non-nil Aligned field") + } + + if *alignment.Aligned != tt.expectAligned { + t.Errorf("Aligned = %v, want %v", *alignment.Aligned, tt.expectAligned) + } + + issuesLen := 0 + if alignment.Issues != nil { + issuesLen = len(*alignment.Issues) + } + if issuesLen != tt.expectIssuesLen { + t.Errorf("Issues length = %d, want %d", issuesLen, tt.expectIssuesLen) } }) } diff --git a/pkg/analyzer/rbl.go b/pkg/analyzer/rbl.go index f13e681..aa35281 100644 --- a/pkg/analyzer/rbl.go +++ b/pkg/analyzer/rbl.go @@ -68,24 +68,15 @@ func NewRBLChecker(timeout time.Duration, rbls []string) *RBLChecker { // RBLResults represents the results of RBL checks type RBLResults struct { - Checks map[string][]RBLCheck // Map of IP -> list of RBL checks for that IP + Checks map[string][]api.BlacklistCheck // Map of IP -> list of RBL checks for that IP IPsChecked []string ListedCount int } -// RBLCheck represents a single RBL check result -// Note: IP is not included here as it's used as the map key in the API -type RBLCheck struct { - RBL string - Listed bool - Response string - Error string -} - // CheckEmail checks all IPs found in the email headers against RBLs func (r *RBLChecker) CheckEmail(email *EmailMessage) *RBLResults { results := &RBLResults{ - Checks: make(map[string][]RBLCheck), + Checks: make(map[string][]api.BlacklistCheck), } // Extract IPs from Received headers @@ -179,15 +170,15 @@ func (r *RBLChecker) isPublicIP(ipStr string) bool { } // checkIP checks a single IP against a single RBL -func (r *RBLChecker) checkIP(ip, rbl string) RBLCheck { - check := RBLCheck{ - RBL: rbl, +func (r *RBLChecker) checkIP(ip, rbl string) api.BlacklistCheck { + check := api.BlacklistCheck{ + Rbl: rbl, } // Reverse the IP for DNSBL query reversedIP := r.reverseIP(ip) if reversedIP == "" { - check.Error = "Failed to reverse IP address" + check.Error = api.PtrTo("Failed to reverse IP address") return check } @@ -208,19 +199,19 @@ func (r *RBLChecker) checkIP(ip, rbl string) RBLCheck { } } // Other DNS errors - check.Error = fmt.Sprintf("DNS lookup failed: %v", err) + check.Error = api.PtrTo(fmt.Sprintf("DNS lookup failed: %v", err)) return check } // If we got a response, check the return code if len(addrs) > 0 { - check.Response = addrs[0] // Return code (e.g., 127.0.0.2) + check.Response = api.PtrTo(addrs[0]) // Return code (e.g., 127.0.0.2) // Check for RBL error codes: 127.255.255.253, 127.255.255.254, 127.255.255.255 // These indicate RBL operational issues, not actual listings if addrs[0] == "127.255.255.253" || addrs[0] == "127.255.255.254" || addrs[0] == "127.255.255.255" { check.Listed = false - check.Error = fmt.Sprintf("RBL %s returned error code %s (RBL operational issue)", rbl, addrs[0]) + check.Error = api.PtrTo(fmt.Sprintf("RBL %s returned error code %s (RBL operational issue)", rbl, addrs[0])) } else { // Normal listing response check.Listed = true @@ -248,8 +239,8 @@ func (r *RBLChecker) reverseIP(ipStr string) string { return fmt.Sprintf("%d.%d.%d.%d", ipv4[3], ipv4[2], ipv4[1], ipv4[0]) } -// GetBlacklistScore calculates the blacklist contribution to deliverability -func (r *RBLChecker) GetBlacklistScore(results *RBLResults) int { +// CalculateRBLScore calculates the blacklist contribution to deliverability +func (r *RBLChecker) CalculateRBLScore(results *RBLResults) int { if results == nil || len(results.IPsChecked) == 0 { // No IPs to check, give benefit of doubt return 100 @@ -258,159 +249,6 @@ func (r *RBLChecker) GetBlacklistScore(results *RBLResults) int { return 100 - results.ListedCount*100/len(r.RBLs) } -// GenerateRBLChecks generates check results for RBL analysis -func (r *RBLChecker) GenerateRBLChecks(results *RBLResults) []api.Check { - var checks []api.Check - - if results == nil { - return checks - } - - // If no IPs were checked, add a warning - if len(results.IPsChecked) == 0 { - checks = append(checks, api.Check{ - Category: api.Blacklist, - Name: "RBL Check", - Status: api.CheckStatusWarn, - Score: 50, - Grade: ScoreToCheckGrade(50), - Message: "No public IP addresses found to check", - Severity: api.PtrTo(api.CheckSeverityLow), - Advice: api.PtrTo("Unable to extract sender IP from email headers"), - }) - return checks - } - - // Create a summary check - summaryCheck := r.generateSummaryCheck(results) - checks = append(checks, summaryCheck) - - // Create individual checks for each listing and RBL errors - for ip, rblChecks := range results.Checks { - for _, check := range rblChecks { - if check.Listed { - detailCheck := r.generateListingCheck(ip, &check) - checks = append(checks, detailCheck) - } else if check.Error != "" && strings.Contains(check.Error, "RBL operational issue") { - // Generate info check for RBL errors - detailCheck := r.generateRBLErrorCheck(ip, &check) - checks = append(checks, detailCheck) - } - } - } - - return checks -} - -// generateSummaryCheck creates an overall RBL summary check -func (r *RBLChecker) generateSummaryCheck(results *RBLResults) api.Check { - check := api.Check{ - Category: api.Blacklist, - Name: "RBL Summary", - } - - score := r.GetBlacklistScore(results) - check.Score = score - check.Grade = ScoreToCheckGrade(score) - - // Calculate total checks across all IPs - totalChecks := 0 - for _, rblChecks := range results.Checks { - totalChecks += len(rblChecks) - } - listedCount := results.ListedCount - - if listedCount == 0 { - check.Status = api.CheckStatusPass - check.Message = fmt.Sprintf("Not listed on any blacklists (%d RBLs checked)", len(r.RBLs)) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("Your sending IP has a good reputation") - } else if listedCount == 1 { - check.Status = api.CheckStatusWarn - check.Message = fmt.Sprintf("Listed on 1 blacklist (out of %d checked)", totalChecks) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("You're listed on one blacklist. Review the specific listing and request delisting if appropriate") - } else if listedCount <= 3 { - check.Status = api.CheckStatusWarn - check.Message = fmt.Sprintf("Listed on %d blacklists (out of %d checked)", listedCount, totalChecks) - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Multiple blacklist listings detected. This will significantly impact deliverability. Review each listing and take corrective action") - } else { - check.Status = api.CheckStatusFail - check.Message = fmt.Sprintf("Listed on %d blacklists (out of %d checked)", listedCount, totalChecks) - check.Severity = api.PtrTo(api.CheckSeverityCritical) - check.Advice = api.PtrTo("Your IP is listed on multiple blacklists. This will severely impact email deliverability. Investigate the cause and request delisting from each RBL") - } - - // Add details about IPs checked - if len(results.IPsChecked) > 0 { - details := fmt.Sprintf("IPs checked: %s", strings.Join(results.IPsChecked, ", ")) - check.Details = &details - } - - return check -} - -// generateListingCheck creates a check for a specific RBL listing -func (r *RBLChecker) generateListingCheck(ip string, rblCheck *RBLCheck) api.Check { - check := api.Check{ - Category: api.Blacklist, - Name: fmt.Sprintf("RBL: %s", rblCheck.RBL), - Status: api.CheckStatusFail, - Score: 0, - Grade: ScoreToCheckGrade(0), - } - - check.Message = fmt.Sprintf("IP %s is listed on %s", ip, rblCheck.RBL) - - // Determine severity based on which RBL - if strings.Contains(rblCheck.RBL, "spamhaus") { - check.Severity = api.PtrTo(api.CheckSeverityCritical) - advice := fmt.Sprintf("Listed on Spamhaus, a widely-used blocklist. Visit https://check.spamhaus.org/ to check details and request delisting") - check.Advice = &advice - } else if strings.Contains(rblCheck.RBL, "spamcop") { - check.Severity = api.PtrTo(api.CheckSeverityHigh) - advice := fmt.Sprintf("Listed on SpamCop. Visit http://www.spamcop.net/bl.shtml to request delisting") - check.Advice = &advice - } else { - check.Severity = api.PtrTo(api.CheckSeverityHigh) - advice := fmt.Sprintf("Listed on %s. Contact the RBL operator for delisting procedures", rblCheck.RBL) - check.Advice = &advice - } - - // Add response code details - if rblCheck.Response != "" { - details := fmt.Sprintf("Response: %s", rblCheck.Response) - check.Details = &details - } - - return check -} - -// generateRBLErrorCheck creates an info-level check for RBL operational errors -func (r *RBLChecker) generateRBLErrorCheck(ip string, rblCheck *RBLCheck) api.Check { - check := api.Check{ - Category: api.Blacklist, - Name: fmt.Sprintf("RBL: %s", rblCheck.RBL), - Status: api.CheckStatusInfo, - Score: 0, // No penalty for RBL operational issues - Grade: ScoreToCheckGrade(-1), - Severity: api.PtrTo(api.CheckSeverityInfo), - } - - check.Message = fmt.Sprintf("RBL %s returned an error code for IP %s", rblCheck.RBL, ip) - - advice := fmt.Sprintf("The RBL %s is experiencing operational issues (error code: %s).", rblCheck.RBL, rblCheck.Response) - check.Advice = &advice - - if rblCheck.Response != "" { - details := fmt.Sprintf("Error code: %s (RBL operational issue, not a listing)", rblCheck.Response) - check.Details = &details - } - - return check -} - // GetUniqueListedIPs returns a list of unique IPs that are listed on at least one RBL func (r *RBLChecker) GetUniqueListedIPs(results *RBLResults) []string { var listedIPs []string @@ -434,7 +272,7 @@ func (r *RBLChecker) GetRBLsForIP(results *RBLResults, ip string) []string { if rblChecks, exists := results.Checks[ip]; exists { for _, check := range rblChecks { if check.Listed { - rbls = append(rbls, check.RBL) + rbls = append(rbls, check.Rbl) } } } diff --git a/pkg/analyzer/rbl_test.go b/pkg/analyzer/rbl_test.go index 2bd5c35..650bc5a 100644 --- a/pkg/analyzer/rbl_test.go +++ b/pkg/analyzer/rbl_test.go @@ -23,11 +23,8 @@ package analyzer import ( "net/mail" - "strings" "testing" "time" - - "git.happydns.org/happyDeliver/internal/api" ) func TestNewRBLChecker(t *testing.T) { @@ -327,7 +324,7 @@ func TestGetBlacklistScore(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - score := checker.GetBlacklistScore(tt.results) + score := checker.CalculateRBLScore(tt.results) if score != tt.expectedScore { t.Errorf("GetBlacklistScore() = %v, want %v", score, tt.expectedScore) } @@ -335,212 +332,6 @@ func TestGetBlacklistScore(t *testing.T) { } } -func TestGenerateSummaryCheck(t *testing.T) { - tests := []struct { - name string - results *RBLResults - expectedStatus api.CheckStatus - expectedScore int - }{ - { - name: "Not listed", - results: &RBLResults{ - IPsChecked: []string{"198.51.100.1"}, - ListedCount: 0, - Checks: map[string][]RBLCheck{ - "198.51.100.1": make([]RBLCheck, 6), // 6 default RBLs - }, - }, - expectedStatus: api.CheckStatusPass, - expectedScore: 200, - }, - { - name: "Listed on 1 RBL", - results: &RBLResults{ - IPsChecked: []string{"198.51.100.1"}, - ListedCount: 1, - Checks: map[string][]RBLCheck{ - "198.51.100.1": make([]RBLCheck, 6), - }, - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 100, - }, - { - name: "Listed on 2 RBLs", - results: &RBLResults{ - IPsChecked: []string{"198.51.100.1"}, - ListedCount: 2, - Checks: map[string][]RBLCheck{ - "198.51.100.1": make([]RBLCheck, 6), - }, - }, - expectedStatus: api.CheckStatusWarn, - expectedScore: 50, - }, - { - name: "Listed on 4+ RBLs", - results: &RBLResults{ - IPsChecked: []string{"198.51.100.1"}, - ListedCount: 4, - Checks: map[string][]RBLCheck{ - "198.51.100.1": make([]RBLCheck, 6), - }, - }, - expectedStatus: api.CheckStatusFail, - expectedScore: 0, - }, - } - - checker := NewRBLChecker(5*time.Second, nil) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := checker.generateSummaryCheck(tt.results) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Score != tt.expectedScore { - t.Errorf("Score = %v, want %v", check.Score, tt.expectedScore) - } - if check.Category != api.Blacklist { - t.Errorf("Category = %v, want %v", check.Category, api.Blacklist) - } - }) - } -} - -func TestGenerateListingCheck(t *testing.T) { - tests := []struct { - name string - rblCheck *RBLCheck - expectedStatus api.CheckStatus - expectedSeverity api.CheckSeverity - }{ - { - name: "Spamhaus listing", - rblCheck: &RBLCheck{ - RBL: "zen.spamhaus.org", - Listed: true, - Response: "127.0.0.2", - }, - expectedStatus: api.CheckStatusFail, - expectedSeverity: api.CheckSeverityCritical, - }, - { - name: "SpamCop listing", - rblCheck: &RBLCheck{ - RBL: "bl.spamcop.net", - Listed: true, - Response: "127.0.0.2", - }, - expectedStatus: api.CheckStatusFail, - expectedSeverity: api.CheckSeverityHigh, - }, - { - name: "Other RBL listing", - rblCheck: &RBLCheck{ - RBL: "dnsbl.sorbs.net", - Listed: true, - Response: "127.0.0.2", - }, - expectedStatus: api.CheckStatusFail, - expectedSeverity: api.CheckSeverityHigh, - }, - } - - checker := NewRBLChecker(5*time.Second, nil) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := checker.generateListingCheck("198.51.100.1", tt.rblCheck) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Severity == nil || *check.Severity != tt.expectedSeverity { - t.Errorf("Severity = %v, want %v", check.Severity, tt.expectedSeverity) - } - if check.Category != api.Blacklist { - t.Errorf("Category = %v, want %v", check.Category, api.Blacklist) - } - if !strings.Contains(check.Name, tt.rblCheck.RBL) { - t.Errorf("Check name should contain RBL name %s", tt.rblCheck.RBL) - } - }) - } -} - -func TestGenerateRBLChecks(t *testing.T) { - tests := []struct { - name string - results *RBLResults - minChecks int - }{ - { - name: "Nil results", - results: nil, - minChecks: 0, - }, - { - name: "No IPs checked", - results: &RBLResults{ - IPsChecked: []string{}, - }, - minChecks: 1, // Warning check - }, - { - name: "Not listed on any RBL", - results: &RBLResults{ - IPsChecked: []string{"198.51.100.1"}, - ListedCount: 0, - Checks: map[string][]RBLCheck{ - "198.51.100.1": { - {RBL: "zen.spamhaus.org", Listed: false}, - {RBL: "bl.spamcop.net", Listed: false}, - }, - }, - }, - minChecks: 1, // Summary check only - }, - { - name: "Listed on 2 RBLs", - results: &RBLResults{ - IPsChecked: []string{"198.51.100.1"}, - ListedCount: 2, - Checks: map[string][]RBLCheck{ - "198.51.100.1": { - {RBL: "zen.spamhaus.org", Listed: true}, - {RBL: "bl.spamcop.net", Listed: true}, - {RBL: "dnsbl.sorbs.net", Listed: false}, - }, - }, - }, - minChecks: 3, // Summary + 2 listing checks - }, - } - - checker := NewRBLChecker(5*time.Second, nil) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := checker.GenerateRBLChecks(tt.results) - - if len(checks) < tt.minChecks { - t.Errorf("Got %d checks, want at least %d", len(checks), tt.minChecks) - } - - // Verify all checks have the Blacklist category - for _, check := range checks { - if check.Category != api.Blacklist { - t.Errorf("Check %s has category %v, want %v", check.Name, check.Category, api.Blacklist) - } - } - }) - } -} - func TestGetUniqueListedIPs(t *testing.T) { results := &RBLResults{ Checks: map[string][]RBLCheck{ diff --git a/pkg/analyzer/report.go b/pkg/analyzer/report.go index 78a0b5e..135dee1 100644 --- a/pkg/analyzer/report.go +++ b/pkg/analyzer/report.go @@ -61,10 +61,11 @@ func NewReportGenerator( type AnalysisResults struct { Email *EmailMessage Authentication *api.AuthenticationResults - SpamAssassin *SpamAssassinResult - DNS *DNSResults - RBL *RBLResults Content *ContentResults + DNS *DNSResults + Headers *api.HeaderAnalysis + RBL *RBLResults + SpamAssassin *SpamAssassinResult } // AnalyzeEmail performs complete email analysis @@ -75,10 +76,11 @@ func (r *ReportGenerator) AnalyzeEmail(email *EmailMessage) *AnalysisResults { // Run all analyzers results.Authentication = r.authAnalyzer.AnalyzeAuthentication(email) - results.SpamAssassin = r.spamAnalyzer.AnalyzeSpamAssassin(email) - results.DNS = r.dnsAnalyzer.AnalyzeDNS(email, results.Authentication) - results.RBL = r.rblChecker.CheckEmail(email) results.Content = r.contentAnalyzer.AnalyzeContent(email) + results.DNS = r.dnsAnalyzer.AnalyzeDNS(email, results.Authentication) + results.Headers = r.headerAnalyzer.GenerateHeaderAnalysis(email) + results.RBL = r.rblChecker.CheckEmail(email) + results.SpamAssassin = r.spamAnalyzer.AnalyzeSpamAssassin(email) return results } @@ -94,77 +96,65 @@ func (r *ReportGenerator) GenerateReport(testID uuid.UUID, results *AnalysisResu CreatedAt: now, } - // Collect all checks from different analyzers - checks := []api.Check{} - - // Authentication checks + // Calculate scores directly from analyzers (no more checks array) + authScore := 0 if results.Authentication != nil { - authChecks := r.authAnalyzer.GenerateAuthenticationChecks(results.Authentication) - checks = append(checks, authChecks...) + authScore = r.authAnalyzer.CalculateAuthenticationScore(results.Authentication) } - // DNS checks - if results.DNS != nil { - dnsChecks := r.dnsAnalyzer.GenerateDNSChecks(results.DNS) - checks = append(checks, dnsChecks...) - } - - // RBL checks - if results.RBL != nil { - rblChecks := r.rblChecker.GenerateRBLChecks(results.RBL) - checks = append(checks, rblChecks...) - } - - // SpamAssassin checks - if results.SpamAssassin != nil { - spamChecks := r.spamAnalyzer.GenerateSpamAssassinChecks(results.SpamAssassin) - checks = append(checks, spamChecks...) - } - - // Content checks + contentScore := 0 if results.Content != nil { - contentChecks := r.contentAnalyzer.GenerateContentChecks(results.Content) - checks = append(checks, contentChecks...) + contentScore = r.contentAnalyzer.CalculateContentScore(results.Content) } - // Header checks - headerChecks := r.headerAnalyzer.GenerateHeaderChecks(results.Email) - checks = append(checks, headerChecks...) - - report.Checks = checks - - // Summarize scores by category - categoryCounts := make(map[api.CheckCategory]int) - categoryTotals := make(map[api.CheckCategory]int) - - for _, check := range checks { - if check.Status == "info" { - continue - } - - categoryCounts[check.Category]++ - categoryTotals[check.Category] += check.Score + headerScore := 0 + if results.Headers != nil { + headerScore = r.headerAnalyzer.CalculateHeaderScore(results.Headers) } - // Calculate mean scores for each category - calcCategoryScore := func(category api.CheckCategory) int { - if count := categoryCounts[category]; count > 0 { - return categoryTotals[category] / count - } - return 0 + blacklistScore := 0 + if results.RBL != nil { + blacklistScore = r.rblChecker.CalculateRBLScore(results.RBL) + } + + spamScore := 0 + if results.SpamAssassin != nil { + spamScore = r.scorer.CalculateSpamScore(results.SpamAssassin) } report.Summary = &api.ScoreSummary{ - AuthenticationScore: calcCategoryScore(api.Authentication), - BlacklistScore: calcCategoryScore(api.Blacklist), - ContentScore: calcCategoryScore(api.Content), - HeaderScore: calcCategoryScore(api.Headers), - SpamScore: calcCategoryScore(api.Spam), + AuthenticationScore: authScore, + BlacklistScore: blacklistScore, + ContentScore: contentScore, + HeaderScore: headerScore, + SpamScore: spamScore, } // Add authentication results report.Authentication = results.Authentication + // Add content analysis + if results.Content != nil { + contentAnalysis := r.contentAnalyzer.GenerateContentAnalysis(results.Content) + report.ContentAnalysis = contentAnalysis + } + + // Add DNS records + if results.DNS != nil { + dnsRecords := r.buildDNSRecords(results.DNS) + if len(dnsRecords) > 0 { + report.DnsRecords = &dnsRecords + } + } + + // Add headers results + report.HeaderAnalysis = results.Headers + + // Add blacklist checks as a map of IP -> array of BlacklistCheck + if results.RBL != nil && len(results.RBL.Checks) > 0 { + report.Blacklists = &results.RBL.Checks + } + // Add SpamAssassin result if results.SpamAssassin != nil { report.Spamassassin = &api.SpamAssassinResult{ @@ -182,39 +172,6 @@ func (r *ReportGenerator) GenerateReport(testID uuid.UUID, results *AnalysisResu } } - // Add DNS records - if results.DNS != nil { - dnsRecords := r.buildDNSRecords(results.DNS) - if len(dnsRecords) > 0 { - report.DnsRecords = &dnsRecords - } - } - - // Add blacklist checks as a map of IP -> array of BlacklistCheck - if results.RBL != nil && len(results.RBL.Checks) > 0 { - blacklistMap := make(map[string][]api.BlacklistCheck) - - // Convert internal RBL checks to API format - for ip, rblChecks := range results.RBL.Checks { - apiChecks := make([]api.BlacklistCheck, 0, len(rblChecks)) - for _, check := range rblChecks { - blCheck := api.BlacklistCheck{ - Rbl: check.RBL, - Listed: check.Listed, - } - if check.Response != "" { - blCheck.Response = &check.Response - } - apiChecks = append(apiChecks, blCheck) - } - blacklistMap[ip] = apiChecks - } - - if len(blacklistMap) > 0 { - report.Blacklists = &blacklistMap - } - } - // Add raw headers if results.Email != nil && results.Email.RawHeaders != "" { report.RawHeaders = &results.Email.RawHeaders diff --git a/pkg/analyzer/report_test.go b/pkg/analyzer/report_test.go index 85edcd2..902f5ff 100644 --- a/pkg/analyzer/report_test.go +++ b/pkg/analyzer/report_test.go @@ -24,7 +24,6 @@ package analyzer import ( "net/mail" "net/textproto" - "strings" "testing" "time" @@ -77,20 +76,6 @@ func TestAnalyzeEmail(t *testing.T) { if results.Authentication == nil { t.Error("Authentication should not be nil") } - - // SpamAssassin might be nil if headers don't exist - // DNS results should exist - // RBL results should exist - // Content results should exist - - if results.Score == nil { - t.Error("Score should not be nil") - } - - // Verify score is within bounds - if results.Score.OverallScore < 0 || results.Score.OverallScore > 100 { - t.Errorf("Overall score %v is out of bounds", results.Score.OverallScore) - } } func TestGenerateReport(t *testing.T) { @@ -125,10 +110,6 @@ func TestGenerateReport(t *testing.T) { t.Error("Summary should not be nil") } - if len(report.Checks) == 0 { - t.Error("Checks should not be empty") - } - // Verify score summary if report.Summary != nil { if report.Summary.AuthenticationScore < 0 || report.Summary.AuthenticationScore > 3 { @@ -147,22 +128,6 @@ func TestGenerateReport(t *testing.T) { t.Errorf("HeaderScore %v is out of bounds", report.Summary.HeaderScore) } } - - // Verify checks have required fields - for i, check := range report.Checks { - if string(check.Category) == "" { - t.Errorf("Check %d: Category should not be empty", i) - } - if check.Name == "" { - t.Errorf("Check %d: Name should not be empty", i) - } - if string(check.Status) == "" { - t.Errorf("Check %d: Status should not be empty", i) - } - if check.Message == "" { - t.Errorf("Check %d: Message should not be empty", i) - } - } } func TestGenerateReportWithSpamAssassin(t *testing.T) { @@ -319,135 +284,6 @@ func TestGenerateRawEmail(t *testing.T) { } } -func TestGetRecommendations(t *testing.T) { - gen := NewReportGenerator(10*time.Second, 10*time.Second, DefaultRBLs) - - tests := []struct { - name string - results *AnalysisResults - expectCount int - }{ - { - name: "Nil results", - results: nil, - expectCount: 0, - }, - { - name: "Results with score", - results: &AnalysisResults{ - Score: &ScoringResult{ - OverallScore: 50, - Grade: ScoreToReportGrade(50), - AuthScore: 15, - SpamScore: 10, - BlacklistScore: 15, - ContentScore: 5, - HeaderScore: 5, - Recommendations: []string{ - "Improve authentication", - "Fix content issues", - }, - }, - }, - expectCount: 2, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - recs := gen.GetRecommendations(tt.results) - if len(recs) != tt.expectCount { - t.Errorf("Got %d recommendations, want %d", len(recs), tt.expectCount) - } - }) - } -} - -func TestGetScoreSummaryText(t *testing.T) { - gen := NewReportGenerator(10*time.Second, 10*time.Second, DefaultRBLs) - - tests := []struct { - name string - results *AnalysisResults - expectEmpty bool - expectString string - }{ - { - name: "Nil results", - results: nil, - expectEmpty: true, - }, - { - name: "Results with score", - results: &AnalysisResults{ - Score: &ScoringResult{ - OverallScore: 85, - Grade: ScoreToReportGrade(85), - AuthScore: 25, - SpamScore: 18, - BlacklistScore: 20, - ContentScore: 15, - HeaderScore: 7, - CategoryBreakdown: map[string]CategoryScore{ - "Authentication": {Score: 25, Status: "Pass"}, - "Spam Filters": {Score: 18, Status: "Pass"}, - "Blacklists": {Score: 20, Status: "Pass"}, - "Content Quality": {Score: 15, Status: "Warn"}, - "Email Structure": {Score: 7, Status: "Warn"}, - }, - }, - }, - expectEmpty: false, - expectString: "8.5/10", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - summary := gen.GetScoreSummaryText(tt.results) - if tt.expectEmpty { - if summary != "" { - t.Errorf("Expected empty summary, got %q", summary) - } - } else { - if summary == "" { - t.Error("Expected non-empty summary") - } - if tt.expectString != "" && !strings.Contains(summary, tt.expectString) { - t.Errorf("Summary should contain %q, got %q", tt.expectString, summary) - } - } - }) - } -} - -func TestReportCategories(t *testing.T) { - gen := NewReportGenerator(10*time.Second, 10*time.Second, DefaultRBLs) - testID := uuid.New() - - email := createComprehensiveTestEmail() - results := gen.AnalyzeEmail(email) - report := gen.GenerateReport(testID, results) - - // Verify all check categories are present - categories := make(map[api.CheckCategory]bool) - for _, check := range report.Checks { - categories[check.Category] = true - } - - expectedCategories := []api.CheckCategory{ - api.Authentication, - api.Dns, - api.Headers, - } - - for _, cat := range expectedCategories { - if !categories[cat] { - t.Errorf("Expected category %s not found in checks", cat) - } - } -} - // Helper functions func createTestEmail() *EmailMessage { @@ -484,21 +320,3 @@ func createTestEmailWithSpamAssassin() *EmailMessage { email.Header[textproto.CanonicalMIMEHeaderKey("X-Spam-Flag")] = []string{"NO"} return email } - -func createComprehensiveTestEmail() *EmailMessage { - email := createTestEmailWithSpamAssassin() - - // Add authentication headers - email.Header[textproto.CanonicalMIMEHeaderKey("Authentication-Results")] = []string{ - "example.com; spf=pass smtp.mailfrom=sender@example.com; dkim=pass header.d=example.com; dmarc=pass", - } - - // Add HTML content - email.Parts = append(email.Parts, MessagePart{ - ContentType: "text/html", - Content: "

Test

Link", - IsHTML: true, - }) - - return email -} diff --git a/pkg/analyzer/scoring.go b/pkg/analyzer/scoring.go index 6db6e0c..3e33e78 100644 --- a/pkg/analyzer/scoring.go +++ b/pkg/analyzer/scoring.go @@ -45,11 +45,6 @@ func ScoreToGrade(score int) string { } } -// ScoreToCheckGrade converts a percentage score to an api.CheckGrade -func ScoreToCheckGrade(score int) api.CheckGrade { - return api.CheckGrade(ScoreToGrade(score)) -} - // ScoreToReportGrade converts a percentage score to an api.ReportGrade func ScoreToReportGrade(score int) api.ReportGrade { return api.ReportGrade(ScoreToGrade(score)) @@ -62,3 +57,29 @@ type DeliverabilityScorer struct{} func NewDeliverabilityScorer() *DeliverabilityScorer { return &DeliverabilityScorer{} } + +// CalculateSpamScore calculates spam score from SpamAssassin results +// Returns a score from 0-100 where higher is better +func (s *DeliverabilityScorer) CalculateSpamScore(result *SpamAssassinResult) int { + if result == nil { + return 100 // No spam scan results, assume good + } + + // SpamAssassin score typically ranges from -10 to +20 + // Score < 0 is very likely ham (good) + // Score 0-5 is threshold range (configurable, usually 5.0) + // Score > 5 is likely spam + + score := result.Score + + // Convert SpamAssassin score to 0-100 scale (inverted - lower SA score is better) + if score <= 0 { + return 100 // Perfect score for ham + } else if score >= result.RequiredScore { + return 0 // Failed spam test + } else { + // Linear scale between 0 and required threshold + percentage := (score / result.RequiredScore) * 100 + return int(100 - percentage) + } +} diff --git a/pkg/analyzer/scoring_test.go b/pkg/analyzer/scoring_test.go index e464432..2cb19c3 100644 --- a/pkg/analyzer/scoring_test.go +++ b/pkg/analyzer/scoring_test.go @@ -21,254 +21,4 @@ package analyzer -import ( - "testing" - - "git.happydns.org/happyDeliver/internal/api" -) - -func TestNewDeliverabilityScorer(t *testing.T) { - scorer := NewDeliverabilityScorer() - if scorer == nil { - t.Fatal("Expected scorer, got nil") - } -} - -func TestIsValidMessageID(t *testing.T) { - tests := []struct { - name string - messageID string - expected bool - }{ - { - name: "Valid Message-ID", - messageID: "", - expected: true, - }, - { - name: "Valid with UUID", - messageID: "<550e8400-e29b-41d4-a716-446655440000@example.com>", - expected: true, - }, - { - name: "Missing angle brackets", - messageID: "abc123@example.com", - expected: false, - }, - { - name: "Missing @ symbol", - messageID: "", - expected: false, - }, - { - name: "Multiple @ symbols", - messageID: "", - expected: false, - }, - { - name: "Empty local part", - messageID: "<@example.com>", - expected: false, - }, - { - name: "Empty domain part", - messageID: "", - expected: false, - }, - { - name: "Empty", - messageID: "", - expected: false, - }, - } - - scorer := NewDeliverabilityScorer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := scorer.isValidMessageID(tt.messageID) - if result != tt.expected { - t.Errorf("isValidMessageID(%q) = %v, want %v", tt.messageID, result, tt.expected) - } - }) - } -} - -func TestCalculateScore(t *testing.T) { - tests := []struct { - name string - authResults *api.AuthenticationResults - spamResult *SpamAssassinResult - rblResults *RBLResults - contentResults *ContentResults - email *EmailMessage - minScore int - maxScore int - expectedGrade string - }{ - { - name: "Perfect email", - authResults: &api.AuthenticationResults{ - Spf: &api.AuthResult{Result: api.AuthResultResultPass}, - Dkim: &[]api.AuthResult{ - {Result: api.AuthResultResultPass}, - }, - Dmarc: &api.AuthResult{Result: api.AuthResultResultPass}, - }, - spamResult: &SpamAssassinResult{ - Score: -1.0, - RequiredScore: 5.0, - }, - rblResults: &RBLResults{ - Checks: []RBLCheck{ - {IP: "192.0.2.1", Listed: false}, - }, - }, - contentResults: &ContentResults{ - HTMLValid: true, - Links: []LinkCheck{{Valid: true, Status: 200}}, - Images: []ImageCheck{{HasAlt: true}}, - HasUnsubscribe: true, - TextPlainRatio: 0.8, - ImageTextRatio: 3.0, - }, - email: &EmailMessage{ - Header: createHeaderWithFields(map[string]string{ - "From": "sender@example.com", - "To": "recipient@example.com", - "Subject": "Test", - "Date": "Mon, 01 Jan 2024 12:00:00 +0000", - "Message-ID": "", - "Reply-To": "reply@example.com", - }), - MessageID: "", - Parts: []MessagePart{{ContentType: "text/plain", Content: "test"}}, - }, - minScore: 90.0, - maxScore: 100.0, - expectedGrade: "A+", - }, - { - name: "Poor email - auth issues", - authResults: &api.AuthenticationResults{ - Spf: &api.AuthResult{Result: api.AuthResultResultFail}, - Dkim: &[]api.AuthResult{}, - Dmarc: nil, - }, - spamResult: &SpamAssassinResult{ - Score: 8.0, - RequiredScore: 5.0, - }, - rblResults: &RBLResults{ - Checks: []RBLCheck{ - { - IP: "192.0.2.1", - RBL: "zen.spamhaus.org", - Listed: true, - }, - }, - ListedCount: 1, - }, - contentResults: &ContentResults{ - HTMLValid: false, - Links: []LinkCheck{{Valid: true, Status: 404}}, - HasUnsubscribe: false, - }, - email: &EmailMessage{ - Header: createHeaderWithFields(map[string]string{ - "From": "sender@example.com", - }), - }, - minScore: 0.0, - maxScore: 50.0, - expectedGrade: "C", - }, - { - name: "Average email", - authResults: &api.AuthenticationResults{ - Spf: &api.AuthResult{Result: api.AuthResultResultPass}, - Dkim: &[]api.AuthResult{ - {Result: api.AuthResultResultPass}, - }, - Dmarc: nil, - }, - spamResult: &SpamAssassinResult{ - Score: 4.0, - RequiredScore: 5.0, - }, - rblResults: &RBLResults{ - Checks: []RBLCheck{ - {IP: "192.0.2.1", Listed: false}, - }, - }, - contentResults: &ContentResults{ - HTMLValid: true, - Links: []LinkCheck{{Valid: true, Status: 200}}, - HasUnsubscribe: false, - }, - email: &EmailMessage{ - Header: createHeaderWithFields(map[string]string{ - "From": "sender@example.com", - "Date": "Mon, 01 Jan 2024 12:00:00 +0000", - "Message-ID": "", - }), - MessageID: "", - Date: "Mon, 01 Jan 2024 12:00:00 +0000", - Parts: []MessagePart{{ContentType: "text/plain", Content: "test"}}, - }, - minScore: 60.0, - maxScore: 90.0, - expectedGrade: "A", - }, - } - - scorer := NewDeliverabilityScorer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := scorer.CalculateScore( - tt.authResults, - tt.spamResult, - tt.rblResults, - tt.contentResults, - tt.email, - ) - - if result == nil { - t.Fatal("Expected result, got nil") - } - - // Check overall score - if result.OverallScore < tt.minScore || result.OverallScore > tt.maxScore { - t.Errorf("OverallScore = %v, want between %v and %v", result.OverallScore, tt.minScore, tt.maxScore) - } - - // Check rating - if result.Grade != api.ReportGrade(tt.expectedGrade) { - t.Errorf("Grade = %q, want %q", result.Grade, tt.expectedGrade) - } - - // Verify score is within bounds - if result.OverallScore < 0.0 || result.OverallScore > 100.0 { - t.Errorf("OverallScore %v is out of bounds [0.0, 100.0]", result.OverallScore) - } - - // Verify category breakdown exists - if len(result.CategoryBreakdown) != 5 { - t.Errorf("Expected 5 categories, got %d", len(result.CategoryBreakdown)) - } - - // Verify recommendations exist - if len(result.Recommendations) == 0 && result.Grade != "A+" { - t.Error("Expected recommendations for non-excellent rating") - } - - // Verify category scores add up to overall score - totalCategoryScore := result.AuthScore + result.SpamScore + result.BlacklistScore + result.ContentScore + result.HeaderScore - if totalCategoryScore != result.OverallScore { - t.Errorf("Category scores sum (%d) doesn't match overall score (%d)", - totalCategoryScore, result.OverallScore) - } - }) - } -} +import () diff --git a/pkg/analyzer/spamassassin.go b/pkg/analyzer/spamassassin.go index a3f175f..9675dbc 100644 --- a/pkg/analyzer/spamassassin.go +++ b/pkg/analyzer/spamassassin.go @@ -22,13 +22,10 @@ package analyzer import ( - "fmt" "math" "regexp" "strconv" "strings" - - "git.happydns.org/happyDeliver/internal/api" ) // SpamAssassinAnalyzer analyzes SpamAssassin results from email headers @@ -198,131 +195,3 @@ func (a *SpamAssassinAnalyzer) GetSpamAssassinScore(result *SpamAssassinResult) // Linear scaling based on how negative/low the score is return 100 - int(math.Round(25*score/required)) } - -// GenerateSpamAssassinChecks generates check results for SpamAssassin analysis -func (a *SpamAssassinAnalyzer) GenerateSpamAssassinChecks(result *SpamAssassinResult) []api.Check { - var checks []api.Check - - if result == nil { - checks = append(checks, api.Check{ - Category: api.Spam, - Name: "SpamAssassin Analysis", - Status: api.CheckStatusWarn, - Score: 0.0, - Grade: ScoreToCheckGrade(0.0), - Message: "No SpamAssassin headers found", - Severity: api.PtrTo(api.CheckSeverityMedium), - Advice: api.PtrTo("Ensure your MTA is configured to run SpamAssassin checks"), - }) - return checks - } - - // Main spam score check - mainCheck := a.generateMainSpamCheck(result) - checks = append(checks, mainCheck) - - // Add checks for significant spam tests (score > 1.0 or < -1.0) - for _, test := range result.Tests { - if detail, ok := result.TestDetails[test]; ok { - if detail.Score > 1.0 || detail.Score < -1.0 { - check := a.generateTestCheck(detail) - checks = append(checks, check) - } - } - } - - return checks -} - -// generateMainSpamCheck creates the main spam score check -func (a *SpamAssassinAnalyzer) generateMainSpamCheck(result *SpamAssassinResult) api.Check { - check := api.Check{ - Category: api.Spam, - Name: "SpamAssassin Score", - } - - score := result.Score - required := result.RequiredScore - if required == 0 { - required = 5.0 - } - - check.Score = a.GetSpamAssassinScore(result) - check.Grade = ScoreToCheckGrade(check.Score) - - // Determine status and message based on score - if score <= 0 { - check.Status = api.CheckStatusPass - check.Message = fmt.Sprintf("Excellent spam score: %.1f (threshold: %.1f)", score, required) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("Your email has a negative spam score, indicating good email practices") - } else if score < required { - check.Status = api.CheckStatusPass - check.Message = fmt.Sprintf("Good spam score: %.1f (threshold: %.1f)", score, required) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Advice = api.PtrTo("Your email passes spam filters") - } else if score < required*1.5 { - check.Status = api.CheckStatusWarn - check.Message = fmt.Sprintf("Borderline spam score: %.1f (threshold: %.1f)", score, required) - check.Severity = api.PtrTo(api.CheckSeverityMedium) - check.Advice = api.PtrTo("Your email is close to being marked as spam. Review the triggered spam tests below") - } else if score < required*2 { - check.Status = api.CheckStatusWarn - check.Message = fmt.Sprintf("High spam score: %.1f (threshold: %.1f)", score, required) - check.Severity = api.PtrTo(api.CheckSeverityHigh) - check.Advice = api.PtrTo("Your email is likely to be marked as spam. Address the issues identified in spam tests") - } else { - check.Status = api.CheckStatusFail - check.Message = fmt.Sprintf("Very high spam score: %.1f (threshold: %.1f)", score, required) - check.Severity = api.PtrTo(api.CheckSeverityCritical) - check.Advice = api.PtrTo("Your email will almost certainly be marked as spam. Urgently address the spam test failures") - } - - // Add details - if len(result.Tests) > 0 { - details := fmt.Sprintf("Triggered %d tests: %s", len(result.Tests), strings.Join(result.Tests[:min(5, len(result.Tests))], ", ")) - if len(result.Tests) > 5 { - details += fmt.Sprintf(" and %d more", len(result.Tests)-5) - } - check.Details = &details - } - - return check -} - -// generateTestCheck creates a check for a specific spam test -func (a *SpamAssassinAnalyzer) generateTestCheck(detail SpamTestDetail) api.Check { - check := api.Check{ - Category: api.Spam, - Name: fmt.Sprintf("Spam Test: %s", detail.Name), - } - - if detail.Score > 0 { - // Negative indicator (increases spam score) - if detail.Score > 2.0 { - check.Status = api.CheckStatusFail - check.Severity = api.PtrTo(api.CheckSeverityHigh) - } else { - check.Status = api.CheckStatusWarn - check.Severity = api.PtrTo(api.CheckSeverityMedium) - } - check.Score = 0.0 - check.Grade = ScoreToCheckGrade(0) - check.Message = fmt.Sprintf("Test failed with score +%.1f", detail.Score) - advice := fmt.Sprintf("%s. This test adds %.1f to your spam score", detail.Description, detail.Score) - check.Advice = &advice - } else { - // Positive indicator (decreases spam score) - check.Status = api.CheckStatusPass - check.Score = 1.0 - check.Grade = ScoreToCheckGrade((1.0 / 20.0) * 100) - check.Severity = api.PtrTo(api.CheckSeverityInfo) - check.Message = fmt.Sprintf("Test passed with score %.1f", detail.Score) - advice := fmt.Sprintf("%s. This test reduces your spam score by %.1f", detail.Description, -detail.Score) - check.Advice = &advice - } - - check.Details = &detail.Description - - return check -} diff --git a/pkg/analyzer/spamassassin_test.go b/pkg/analyzer/spamassassin_test.go index 54b9c0c..2ed2890 100644 --- a/pkg/analyzer/spamassassin_test.go +++ b/pkg/analyzer/spamassassin_test.go @@ -26,8 +26,6 @@ import ( "net/mail" "strings" "testing" - - "git.happydns.org/happyDeliver/internal/api" ) func TestParseSpamStatus(t *testing.T) { @@ -298,86 +296,6 @@ func TestAnalyzeSpamAssassin(t *testing.T) { } } -func TestGenerateSpamAssassinChecks(t *testing.T) { - tests := []struct { - name string - result *SpamAssassinResult - expectedStatus api.CheckStatus - minChecks int - }{ - { - name: "Nil result", - result: nil, - expectedStatus: api.CheckStatusWarn, - minChecks: 1, - }, - { - name: "Clean email", - result: &SpamAssassinResult{ - IsSpam: false, - Score: -0.5, - RequiredScore: 5.0, - Tests: []string{"ALL_TRUSTED"}, - TestDetails: map[string]SpamTestDetail{ - "ALL_TRUSTED": { - Name: "ALL_TRUSTED", - Score: -1.5, - Description: "All mail servers are trusted", - }, - }, - }, - expectedStatus: api.CheckStatusPass, - minChecks: 2, // Main check + one test detail - }, - { - name: "Spam email", - result: &SpamAssassinResult{ - IsSpam: true, - Score: 15.0, - RequiredScore: 5.0, - Tests: []string{"BAYES_99", "SPOOFED_SENDER"}, - TestDetails: map[string]SpamTestDetail{ - "BAYES_99": { - Name: "BAYES_99", - Score: 5.0, - Description: "Bayes spam probability is 99 to 100%", - }, - "SPOOFED_SENDER": { - Name: "SPOOFED_SENDER", - Score: 3.5, - Description: "From address doesn't match envelope sender", - }, - }, - }, - expectedStatus: api.CheckStatusFail, - minChecks: 3, // Main check + two significant tests - }, - } - - analyzer := NewSpamAssassinAnalyzer() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - checks := analyzer.GenerateSpamAssassinChecks(tt.result) - - if len(checks) < tt.minChecks { - t.Errorf("Got %d checks, want at least %d", len(checks), tt.minChecks) - } - - // Check main check (first one) - if len(checks) > 0 { - mainCheck := checks[0] - if mainCheck.Status != tt.expectedStatus { - t.Errorf("Main check status = %v, want %v", mainCheck.Status, tt.expectedStatus) - } - if mainCheck.Category != api.Spam { - t.Errorf("Main check category = %v, want %v", mainCheck.Category, api.Spam) - } - } - }) - } -} - func TestAnalyzeSpamAssassinNoHeaders(t *testing.T) { analyzer := NewSpamAssassinAnalyzer() email := &EmailMessage{ @@ -391,98 +309,6 @@ func TestAnalyzeSpamAssassinNoHeaders(t *testing.T) { } } -func TestGenerateMainSpamCheck(t *testing.T) { - analyzer := NewSpamAssassinAnalyzer() - - tests := []struct { - name string - score float64 - required float64 - expectedStatus api.CheckStatus - }{ - {"Excellent", -1.0, 5.0, api.CheckStatusPass}, - {"Good", 2.0, 5.0, api.CheckStatusPass}, - {"Borderline", 6.0, 5.0, api.CheckStatusWarn}, - {"High", 8.0, 5.0, api.CheckStatusWarn}, - {"Very High", 15.0, 5.0, api.CheckStatusFail}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := &SpamAssassinResult{ - Score: tt.score, - RequiredScore: tt.required, - } - - check := analyzer.generateMainSpamCheck(result) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Category != api.Spam { - t.Errorf("Category = %v, want %v", check.Category, api.Spam) - } - if !strings.Contains(check.Message, "spam score") { - t.Error("Message should contain 'spam score'") - } - }) - } -} - -func TestGenerateTestCheck(t *testing.T) { - analyzer := NewSpamAssassinAnalyzer() - - tests := []struct { - name string - detail SpamTestDetail - expectedStatus api.CheckStatus - }{ - { - name: "High penalty test", - detail: SpamTestDetail{ - Name: "BAYES_99", - Score: 5.0, - Description: "Bayes spam probability is 99 to 100%", - }, - expectedStatus: api.CheckStatusFail, - }, - { - name: "Medium penalty test", - detail: SpamTestDetail{ - Name: "HTML_MESSAGE", - Score: 1.5, - Description: "Contains HTML", - }, - expectedStatus: api.CheckStatusWarn, - }, - { - name: "Positive test", - detail: SpamTestDetail{ - Name: "ALL_TRUSTED", - Score: -2.0, - Description: "All mail servers are trusted", - }, - expectedStatus: api.CheckStatusPass, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - check := analyzer.generateTestCheck(tt.detail) - - if check.Status != tt.expectedStatus { - t.Errorf("Status = %v, want %v", check.Status, tt.expectedStatus) - } - if check.Category != api.Spam { - t.Errorf("Category = %v, want %v", check.Category, api.Spam) - } - if !strings.Contains(check.Name, tt.detail.Name) { - t.Errorf("Check name should contain test name %s", tt.detail.Name) - } - }) - } -} - const sampleEmailWithSpamassassinHeader = `X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on e4a8b8eb87ec X-Spam-Status: No, score=-0.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, @@ -623,34 +449,6 @@ func TestAnalyzeRealEmailExample(t *testing.T) { if score != 100 { t.Errorf("GetSpamAssassinScore() = %v, want 100 (excellent score for negative spam score)", score) } - - // Test GenerateSpamAssassinChecks - checks := analyzer.GenerateSpamAssassinChecks(result) - if len(checks) < 1 { - t.Fatal("Expected at least 1 check, got none") - } - - // Main check should be PASS with excellent score - mainCheck := checks[0] - if mainCheck.Status != api.CheckStatusPass { - t.Errorf("Main check status = %v, want %v", mainCheck.Status, api.CheckStatusPass) - } - if mainCheck.Category != api.Spam { - t.Errorf("Main check category = %v, want %v", mainCheck.Category, api.Spam) - } - if !strings.Contains(mainCheck.Message, "spam score") { - t.Errorf("Main check message should contain 'spam score', got: %s", mainCheck.Message) - } - if mainCheck.Score != 100 { - t.Errorf("Main check score = %v, want 100", mainCheck.Score) - } - - // Log all checks for debugging - t.Logf("Generated %d checks:", len(checks)) - for i, check := range checks { - t.Logf(" Check %d: %s - %s (score: %d, status: %s)", - i+1, check.Name, check.Message, check.Score, check.Status) - } } // Helper function to compare string slices diff --git a/web/src/lib/components/AuthenticationCard.svelte b/web/src/lib/components/AuthenticationCard.svelte new file mode 100644 index 0000000..ec043cf --- /dev/null +++ b/web/src/lib/components/AuthenticationCard.svelte @@ -0,0 +1,210 @@ + + +
+
+

+ + + Authentication + + {#if authenticationScore !== undefined} + + {authenticationScore}% + + {/if} +

+
+
+
+ +
+
+ {#if authentication.spf} + +
+ SPF + + {authentication.spf.result} + + {#if authentication.spf.domain} +
+ Domain: + {authentication.spf.domain} +
+ {/if} + {#if authentication.spf.details} + 
{authentication.spf.details}
+ {/if} +
+ {:else} + +
+ SPF + + {getAuthResultText('missing')} + +
SPF record is required for proper email authentication
+
+ {/if} +
+
+ + +
+
+ {#if authentication.dkim && authentication.dkim.length > 0} + +
+ DKIM + + {authentication.dkim[0].result} + + {#if authentication.dkim[0].domain} +
{authentication.dkim[0].domain}
+ {/if} + {#if authentication.dkim[0].selector} +
Selector: {authentication.dkim[0].selector}
+ {/if} + {#if authentication.dkim.details} + 
{authentication.dkim.details}
+ {/if} +
+ {:else} + +
+ DKIM + + {getAuthResultText('missing')} + +
DKIM signature is required for proper email authentication
+
+ {/if} +
+
+ + +
+
+ {#if authentication.dmarc} + +
+ DMARC + + {authentication.dmarc.result} + + {#if authentication.dmarc.details} + 
{authentication.dmarc.details}
+ {/if} +
+ {:else} + +
+ DMARC + + {getAuthResultText('missing')} + +
DMARC policy is required for proper email authentication
+
+ {/if} +
+
+ + +
+
+ {#if authentication.bimi} + +
+ BIMI + + {authentication.bimi.result} + + {#if authentication.bimi.details} + 
{authentication.bimi.details}
+ {/if} +
+ {:else} + +
+ BIMI + + Optional + +
Brand Indicators for Message Identification (optional enhancement)
+
+ {/if} +
+
+ + + {#if authentication.arc} +
+
+ +
+ ARC + + {authentication.arc.result} + + {#if authentication.arc.chain_length} +
Chain length: {authentication.arc.chain_length}
+ {/if} + {#if authentication.arc.details} + 
{authentication.arc.details}
+ {/if} +
+
+
+ {/if} +
+
+
diff --git a/web/src/lib/components/BlacklistCard.svelte b/web/src/lib/components/BlacklistCard.svelte new file mode 100644 index 0000000..4794599 --- /dev/null +++ b/web/src/lib/components/BlacklistCard.svelte @@ -0,0 +1,60 @@ + + +
+
+

+ + + Blacklist Checks + + {#if blacklistScore !== undefined} + + {blacklistScore}% + + {/if} +

+
+
+
+ {#each Object.entries(blacklists) as [ip, checks]} +
+
+ + {ip} +
+
+ + + + + + + + + {#each checks as check} + + + + + {/each} + +
RBLStatus
{check.rbl} + + {check.error ? 'Error' : (check.listed ? 'Listed' : 'Clean')} + +
+
+
+ {/each} +
+
+
diff --git a/web/src/lib/components/CheckCard.svelte b/web/src/lib/components/CheckCard.svelte deleted file mode 100644 index de84a70..0000000 --- a/web/src/lib/components/CheckCard.svelte +++ /dev/null @@ -1,71 +0,0 @@ - - -
-
-
-
- -
-
-
-
{check.name}
- {check.score}% -
- -

{check.message}

- - {#if check.advice} - - {/if} - - {#if check.details} -
- Technical Details - 
{check.details}
-
- {/if} -
-
-
-
- - diff --git a/web/src/lib/components/ContentAnalysisCard.svelte b/web/src/lib/components/ContentAnalysisCard.svelte new file mode 100644 index 0000000..6ee4cbb --- /dev/null +++ b/web/src/lib/components/ContentAnalysisCard.svelte @@ -0,0 +1,165 @@ + + +
+
+

+ + + Content Analysis + + {#if contentScore !== undefined} + + {contentScore}% + + {/if} +

+
+
+
+
+
+ + HTML Part +
+
+ + Plaintext Part +
+ {#if typeof contentAnalysis.has_unsubscribe_link === 'boolean'} +
+ + Unsubscribe Link +
+ {/if} +
+
+ {#if contentAnalysis.text_to_image_ratio !== undefined} +
+ Text to Image Ratio: + {contentAnalysis.text_to_image_ratio.toFixed(2)} +
+ {/if} + {#if contentAnalysis.unsubscribe_methods && contentAnalysis.unsubscribe_methods.length > 0} +
+ Unsubscribe Methods: +
+ {#each contentAnalysis.unsubscribe_methods as method} + {method} + {/each} +
+
+ {/if} +
+
+ + {#if contentAnalysis.html_issues && contentAnalysis.html_issues.length > 0} +
+
Content Issues
+ {#each contentAnalysis.html_issues as issue} +
+
+
+ {issue.type} +
{issue.message}
+ {#if issue.location} +
{issue.location}
+ {/if} + {#if issue.advice} +
+ + {issue.advice} +
+ {/if} +
+ {issue.severity} +
+
+ {/each} +
+ {/if} + + {#if contentAnalysis.links && contentAnalysis.links.length > 0} +
+
Links ({contentAnalysis.links.length})
+
+ + + + + + + + + + {#each contentAnalysis.links as link} + + + + + + {/each} + +
URLStatusHTTP Code
+ {link.url} + {#if link.is_shortened} + Shortened + {/if} + + + {link.status} + + {link.http_code || '-'}
+
+
+ {/if} + + {#if contentAnalysis.images && contentAnalysis.images.length > 0} +
+
Images ({contentAnalysis.images.length})
+
+ + + + + + + + + + {#each contentAnalysis.images as image} + + + + + + {/each} + +
SourceAlt TextTracking
{image.src || '-'} + {#if image.has_alt} + + {image.alt_text || 'Present'} + {:else} + + Missing + {/if} + + {#if image.is_tracking_pixel} + Tracking Pixel + {:else} + - + {/if} +
+
+
+ {/if} +
+
diff --git a/web/src/lib/components/DnsRecordsCard.svelte b/web/src/lib/components/DnsRecordsCard.svelte new file mode 100644 index 0000000..79272f5 --- /dev/null +++ b/web/src/lib/components/DnsRecordsCard.svelte @@ -0,0 +1,46 @@ + + +
+
+

+ + DNS Records +

+
+
+
+ + + + + + + + + + + {#each dnsRecords as record} + + + + + + + {/each} + +
DomainTypeStatusValue
{record.domain}{record.record_type} + + {record.status} + + {record.value || '-'}
+
+
+
diff --git a/web/src/lib/components/HeaderAnalysisCard.svelte b/web/src/lib/components/HeaderAnalysisCard.svelte new file mode 100644 index 0000000..6b03966 --- /dev/null +++ b/web/src/lib/components/HeaderAnalysisCard.svelte @@ -0,0 +1,169 @@ + + +
+
+

+ + + Header Analysis + + {#if headerScore !== undefined} + + {headerScore}% + + {/if} +

+
+
+ {#if headerAnalysis.issues && headerAnalysis.issues.length > 0} +
+
Issues
+ {#each headerAnalysis.issues as issue} +
+
+
+ {issue.header} +
{issue.message}
+ {#if issue.advice} +
+ + {issue.advice} +
+ {/if} +
+ {issue.severity} +
+
+ {/each} +
+ {/if} + + {#if headerAnalysis.domain_alignment} +
+
Domain Alignment
+
+
+
+
+ From Domain +
{headerAnalysis.domain_alignment.from_domain || '-'}
+
+
+ Return-Path Domain +
{headerAnalysis.domain_alignment.return_path_domain || '-'}
+
+
+ Aligned +
+ + {headerAnalysis.domain_alignment.aligned ? 'Yes' : 'No'} +
+
+
+ {#if headerAnalysis.domain_alignment.issues && headerAnalysis.domain_alignment.issues.length > 0} +
+ {#each headerAnalysis.domain_alignment.issues as issue} +
+ + {issue} +
+ {/each} +
+ {/if} +
+
+
+ {/if} + + {#if headerAnalysis.headers && Object.keys(headerAnalysis.headers).length > 0} +
+
Headers
+
+ + + + + + + + + + + {#each Object.entries(headerAnalysis.headers) as [name, check]} + + + + + + + {/each} + +
HeaderPresentValidValue
+ {name} + {#if check.importance} + + {check.importance} + + {/if} + + + + {#if check.valid !== undefined} + + {:else} + - + {/if} + + {check.value || '-'} + {#if check.issues && check.issues.length > 0} + {#each check.issues as issue} +
+ + {issue} +
+ {/each} + {/if} +
+
+
+ {/if} + + {#if headerAnalysis.received_chain && headerAnalysis.received_chain.length > 0} +
+
Email Path (Received Chain)
+
+ {#each headerAnalysis.received_chain as hop, i} +
+
+
+ {i + 1} + {hop.from || 'Unknown'} → {hop.by || 'Unknown'} +
+ {hop.timestamp || '-'} +
+ {#if hop.with || hop.id} +

+ {#if hop.with} + Protocol: {hop.with} + {/if} + {#if hop.id} + ID: {hop.id} + {/if} +

+ {/if} +
+ {/each} +
+
+ {/if} +
+
diff --git a/web/src/lib/components/ScoreCard.svelte b/web/src/lib/components/ScoreCard.svelte index 0b74a38..fb0912f 100644 --- a/web/src/lib/components/ScoreCard.svelte +++ b/web/src/lib/components/ScoreCard.svelte @@ -50,19 +50,6 @@ Authentication -
-
- = 100} - class:text-warning={summary.spam_score < 100 && summary.spam_score >= 50} - class:text-danger={summary.spam_score < 50} - > - {summary.spam_score}% - - Spam Score -
-
Blacklists
-
-
- = 100} - class:text-warning={summary.content_score < 100 && - summary.content_score >= 50} - class:text-danger={summary.content_score < 50} - > - {summary.content_score}% - - Content -
-
Headers
+
+
+ = 100} + class:text-warning={summary.spam_score < 100 && summary.spam_score >= 50} + class:text-danger={summary.spam_score < 50} + > + {summary.spam_score}% + + Spam Score +
+
+
+
+ = 100} + class:text-warning={summary.content_score < 100 && + summary.content_score >= 50} + class:text-danger={summary.content_score < 50} + > + {summary.content_score}% + + Content +
+
{/if} diff --git a/web/src/lib/components/index.ts b/web/src/lib/components/index.ts index 8da4188..a5b56ae 100644 --- a/web/src/lib/components/index.ts +++ b/web/src/lib/components/index.ts @@ -2,7 +2,11 @@ export { default as FeatureCard } from "./FeatureCard.svelte"; export { default as HowItWorksStep } from "./HowItWorksStep.svelte"; export { default as ScoreCard } from "./ScoreCard.svelte"; -export { default as CheckCard } from "./CheckCard.svelte"; export { default as SpamAssassinCard } from "./SpamAssassinCard.svelte"; export { default as EmailAddressDisplay } from "./EmailAddressDisplay.svelte"; export { default as PendingState } from "./PendingState.svelte"; +export { default as AuthenticationCard } from "./AuthenticationCard.svelte"; +export { default as DnsRecordsCard } from "./DnsRecordsCard.svelte"; +export { default as BlacklistCard } from "./BlacklistCard.svelte"; +export { default as ContentAnalysisCard } from "./ContentAnalysisCard.svelte"; +export { default as HeaderAnalysisCard } from "./HeaderAnalysisCard.svelte"; diff --git a/web/src/routes/test/[test]/+page.svelte b/web/src/routes/test/[test]/+page.svelte index fd36ce7..0c52dd0 100644 --- a/web/src/routes/test/[test]/+page.svelte +++ b/web/src/routes/test/[test]/+page.svelte @@ -3,7 +3,16 @@ import { page } from "$app/state"; import { getTest, getReport, reanalyzeReport } from "$lib/api"; import type { Test, Report } from "$lib/api/types.gen"; - import { ScoreCard, CheckCard, SpamAssassinCard, PendingState } from "$lib/components"; + import { + ScoreCard, + SpamAssassinCard, + PendingState, + AuthenticationCard, + DnsRecordsCard, + BlacklistCard, + ContentAnalysisCard, + HeaderAnalysisCard + } from "$lib/components"; let testId = $derived(page.params.test); let test = $state(null); @@ -15,20 +24,6 @@ let nextfetch = $state(23); let nbfetch = $state(0); - // Group checks by category - let groupedChecks = $derived(() => { - if (!report) return { }; - - const groups: Record = { }; - for (const check of report.checks) { - if (!groups[check.category]) { - groups[check.category] = []; - } - groups[check.category].push(check); - } - return groups; - }); - async function fetchTest() { if (nbfetch > 0) { nextfetch = Math.max(nextfetch, Math.floor(3 + nbfetch * 0.5)); @@ -86,29 +81,6 @@ stopPolling(); }); - function getCategoryIcon(category: string): string { - switch (category) { - case "authentication": - return "bi-shield-check"; - case "dns": - return "bi-diagram-3"; - case "content": - return "bi-file-text"; - case "blacklist": - return "bi-shield-exclamation"; - case "headers": - return "bi-list-ul"; - case "spam": - return "bi-filter"; - default: - return "bi-question-circle"; - } - } - - function getCategoryScore(checks: typeof report.checks): number { - return Math.round(checks.reduce((sum, check) => sum + check.score, 0) / checks.filter((c) => c.status != "info").length); - } - function getScoreColorClass(percentage: number): string { if (percentage >= 80) return "text-success"; if (percentage >= 50) return "text-warning"; @@ -166,45 +138,78 @@
-
+
- -
-
-

Detailed Checks

- {#each Object.entries(groupedChecks()) as [category, checks]} - {@const categoryScore = getCategoryScore(checks)} -
-

- - - {category} - - - {categoryScore}% - -

- {#each checks as check} - - {/each} -
- {/each} + + {#if report.dns_records && report.dns_records.length > 0} +
+
+ +
-
+ {/if} + + + {#if report.authentication} +
+
+ +
+
+ {/if} + + + {#if report.blacklists && Object.keys(report.blacklists).length > 0} +
+
+ +
+
+ {/if} + + + {#if report.header_analysis} + + {/if} {#if report.spamassassin} -
+
{/if} + + {#if report.content_analysis} +
+
+ +
+
+ {/if} +