Gondulf/docs/designs/dns-verification-bug-fix.md

# DNS Verification Bug Fix Design

## Purpose
Fix critical bug in DNS TXT record verification where the code queries the wrong domain location, preventing successful domain verification even when users have correctly configured their DNS records.

## Problem Statement

### Current Incorrect Behavior
The DNS verification service currently queries the wrong domain for TXT records:

1. **User instructions** (correctly shown in template): Set TXT record at `_gondulf.{domain}`
2. **User action**: Creates TXT record at `_gondulf.thesatelliteoflove.com` with value `gondulf-verify-domain`
3. **Code behavior** (INCORRECT): Queries `thesatelliteoflove.com` instead of `_gondulf.thesatelliteoflove.com`
4. **Result**: Verification always fails

### Root Cause
In `src/gondulf/dns.py`, the `verify_txt_record` method passes the domain directly to `get_txt_records`, which then queries that exact domain. The calling code in `src/gondulf/routers/authorization.py` also passes just the base domain without the `_gondulf.` prefix.

## Design Overview

The fix requires modifying the DNS verification logic to correctly prefix the domain with `_gondulf.` when querying TXT records for Gondulf domain verification purposes.

## Component Details

### 1. DNSService Updates (`src/gondulf/dns.py`)

#### Option A: Modify `verify_txt_record` Method (RECOMMENDED)
Update the `verify_txt_record` method to handle Gondulf-specific verification by prefixing the domain:

```python
def verify_txt_record(self, domain: str, expected_value: str) -> bool:
    """
    Verify that domain has a TXT record with the expected value.

    For Gondulf domain verification (expected_value="gondulf-verify-domain"),
    queries the _gondulf.{domain} subdomain as per specification.

    Args:
        domain: Domain name to verify (e.g., "example.com")
        expected_value: Expected TXT record value

    Returns:
        True if expected value found in TXT records, False otherwise
    """
    try:
        # For Gondulf domain verification, query _gondulf subdomain
        if expected_value == "gondulf-verify-domain":
            query_domain = f"_gondulf.{domain}"
        else:
            query_domain = domain

        txt_records = self.get_txt_records(query_domain)

        # Check if expected value is in any TXT record
        for record in txt_records:
            if expected_value in record:
                logger.info(
                    f"TXT record verification successful for domain={domain} "
                    f"(queried {query_domain})"
                )
                return True

        logger.debug(
            f"TXT record verification failed: expected value not found "
            f"for domain={domain} (queried {query_domain})"
        )
        return False

    except DNSError as e:
        logger.warning(f"TXT record verification failed for domain={domain}: {e}")
        return False
```

#### Option B: Create Dedicated Method (ALTERNATIVE - NOT RECOMMENDED)
Add a new method specifically for Gondulf verification:

```python
def verify_gondulf_domain(self, domain: str) -> bool:
    """
    Verify Gondulf domain ownership via TXT record at _gondulf.{domain}.

    Args:
        domain: Domain name to verify (e.g., "example.com")

    Returns:
        True if gondulf-verify-domain found in _gondulf.{domain} TXT records
    """
    gondulf_subdomain = f"_gondulf.{domain}"
    return self.verify_txt_record(gondulf_subdomain, "gondulf-verify-domain")
```

**Recommendation**: Use Option A. It keeps the fix localized to the DNS service and maintains backward compatibility while fixing the bug with minimal changes.

### 2. No Changes Required in Authorization Router

With Option A, no changes are needed in `src/gondulf/routers/authorization.py` since the fix is entirely contained within the DNS service. The existing call remains correct:

```python
dns_verified = dns_service.verify_txt_record(domain, "gondulf-verify-domain")
```

### 3. Template Remains Correct

The template (`src/gondulf/templates/verification_error.html`) already shows the correct instructions and needs no changes.

## Data Models

No data model changes required.

## API Contracts

No API changes required. This is an internal bug fix.

## Error Handling

### DNS Query Errors
The existing error handling in `get_txt_records` is sufficient:
- NXDOMAIN: Domain doesn't exist (including subdomain)
- NoAnswer: No TXT records found
- Timeout: DNS server timeout
- Other DNS exceptions: General failure

All these cases correctly return False for verification failure.

### Logging Updates
Update log messages to include which domain was actually queried:
- Success: Include both the requested domain and the queried domain
- Failure: Include both domains to aid debugging

## Security Considerations

1. **No New Attack Vectors**: The fix doesn't introduce new security concerns
2. **DNS Rebinding**: Not applicable (we're only reading TXT records)
3. **Cache Poisoning**: Existing DNS resolver safeguards apply
4. **Subdomain Takeover**: The `_gondulf` prefix is specifically chosen to avoid conflicts

## Testing Strategy

### Unit Tests Required

1. **Test Gondulf domain verification with correct TXT record**
   - Mock DNS response for `_gondulf.example.com` with value `gondulf-verify-domain`
   - Verify `verify_txt_record("example.com", "gondulf-verify-domain")` returns True

2. **Test Gondulf domain verification with missing TXT record**
   - Mock DNS response for `_gondulf.example.com` with no TXT records
   - Verify `verify_txt_record("example.com", "gondulf-verify-domain")` returns False

3. **Test Gondulf domain verification with wrong TXT value**
   - Mock DNS response for `_gondulf.example.com` with value `wrong-value`
   - Verify `verify_txt_record("example.com", "gondulf-verify-domain")` returns False

4. **Test non-Gondulf TXT verification still works**
   - Mock DNS response for `example.com` (not prefixed) with value `other-value`
   - Verify `verify_txt_record("example.com", "other-value")` returns True
   - Ensures backward compatibility for any other TXT verification uses

5. **Test NXDOMAIN handling**
   - Mock NXDOMAIN for `_gondulf.example.com`
   - Verify `verify_txt_record("example.com", "gondulf-verify-domain")` returns False

### Integration Test

1. **End-to-end authorization flow test**
   - Set up test domain with `_gondulf.{domain}` TXT record
   - Attempt authorization flow
   - Verify DNS verification passes

### Manual Testing

1. Configure real DNS record: `_gondulf.yourdomain.com` with value `gondulf-verify-domain`
2. Test authorization flow
3. Verify successful DNS verification
4. Check logs show correct domain being queried

## Acceptance Criteria

1. ✅ DNS verification queries `_gondulf.{domain}` when verifying Gondulf domain ownership
2. ✅ Users with correctly configured TXT records can successfully verify their domain
3. ✅ Log messages clearly show which domain was queried for debugging
4. ✅ Non-Gondulf TXT verification (if used elsewhere) continues to work
5. ✅ All existing tests pass
6. ✅ New unit tests cover the fix
7. ✅ Manual testing confirms real DNS records work

## Implementation Notes

1. **Critical Bug**: This is a P0 bug that completely breaks domain verification
2. **Simple Fix**: The fix is straightforward - just add the prefix when appropriate
3. **Test Thoroughly**: While the fix is simple, ensure comprehensive testing
4. **Verify Logs**: Update logging to be clear about what domain is being queried

## Migration Considerations

None required. This is a bug fix that makes the code work as originally intended. No database migrations or data changes needed.