fix(templates): handle optional PKCE in retry links

The "Request a new code" and "Try Again" links were outputting
code_challenge=None when PKCE was not used, causing the retry
to fail with validation errors.

Now only includes PKCE parameters if code_challenge has a value.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

client_id_validation_analysis.md

@@ -0,0 +1,187 @@
+# Client ID Validation Compliance Analysis
+
+## W3C IndieAuth Specification Requirements (Section 3.2)
+
+According to the W3C IndieAuth specification, client identifiers MUST meet the following requirements:
+
+1. **Scheme**: MUST have either `https` or `http` scheme
+2. **Path**: MUST contain a path component
+3. **Path Segments**: MUST NOT contain single-dot or double-dot path segments
+4. **Query String**: MAY contain a query string component
+5. **Fragment**: MUST NOT contain a fragment component
+6. **User Info**: MUST NOT contain username or password component
+7. **Port**: MAY contain a port
+8. **Hostname**:
+   - MUST be domain names or a loopback interface
+   - MUST NOT be IPv4 or IPv6 addresses except for 127.0.0.1 or [::1]
+
+## Current Gondulf Implementation Analysis
+
+### What We Currently Validate ✅
+
+1. **Scheme Validation** (PARTIAL)
+   - ✅ We require HTTPS scheme in `normalize_client_id()`
+   - ❌ We reject HTTP completely, even for localhost (spec allows HTTP)
+   - Location: `/src/gondulf/utils/validation.py` lines 48-49
+
+2. **Query String Preservation**
+   - ✅ We preserve query strings correctly
+   - Location: `/src/gondulf/utils/validation.py` lines 58-59
+
+3. **Fragment Preservation** (BUT NOT VALIDATED)
+   - ⚠️ We preserve fragments but don't reject them
+   - Location: `/src/gondulf/utils/validation.py` lines 60-61
+
+### What We DON'T Validate ❌
+
+1. **Path Component Requirement**
+   - ❌ We don't verify that a path component exists
+   - Spec requires at least "/" as a path
+
+2. **Path Segment Validation**
+   - ❌ We don't check for single-dot (.) or double-dot (..) path segments
+   - These should be rejected per spec
+
+3. **Fragment Component**
+   - ❌ We don't reject URLs with fragments (we actually preserve them)
+   - Spec says MUST NOT contain fragment
+
+4. **User Info Component**
+   - ❌ We don't check for or reject username:password in URLs
+   - Spec says MUST NOT contain username or password
+
+5. **Hostname/IP Validation**
+   - ❌ We don't validate that hostnames are domain names
+   - ❌ We don't reject IPv4/IPv6 addresses (except 127.0.0.1 and [::1])
+   - ❌ We don't have special handling for localhost/127.0.0.1/[::1]
+
+6. **Port Support**
+   - ✅ We preserve ports correctly
+   - ⚠️ But we don't allow HTTP for localhost with ports
+
+## Reference Implementation (IndieLogin) Comparison
+
+From examining `/home/phil/Projects/indielogin.com/app/Authenticate.php`:
+
+1. Uses `\p3k\url\is_url()` for basic URL validation (line 49)
+2. Checks that client_id contains a dot OR is localhost (line 51)
+3. Validates redirect_uri domain matches client_id domain (lines 75-95)
+4. Does NOT appear to validate all spec requirements either
+
+## Non-Compliance Issues Found
+
+### Critical Issues
+
+1. **HTTP localhost support missing**
+   ```python
+   # Current code rejects all HTTP
+   if parsed.scheme != 'https':
+       raise ValueError("client_id must use https scheme")
+   ```
+   Should allow HTTP for localhost/127.0.0.1/[::1]
+
+2. **Fragment rejection missing**
+   ```python
+   # Current code preserves fragments instead of rejecting them
+   if parsed.fragment:
+       normalized += f"#{parsed.fragment}"
+   ```
+
+3. **Path component not validated**
+   - No check that path exists (at minimum "/")
+
+4. **IP address validation missing**
+   - Should reject IPs except 127.0.0.1 and [::1]
+
+### Security Implications
+
+1. **Fragment acceptance** - Could lead to confusion about actual client_id
+2. **User info not rejected** - Could expose credentials in logs
+3. **IP addresses allowed** - Could bypass domain validation
+4. **Path traversal** - Accepting ../.. could be security issue
+
+## Recommended Fixes
+
+### Priority 1: Add proper client_id validation function
+
+Create a new `validate_client_id()` function that checks ALL spec requirements:
+
+```python
+def validate_client_id(client_id: str) -> tuple[bool, str]:
+    """
+    Validate client_id per W3C IndieAuth spec Section 3.2.
+
+    Returns:
+        (is_valid, error_message)
+    """
+    try:
+        parsed = urlparse(client_id)
+
+        # 1. Check scheme
+        if parsed.scheme not in ['https', 'http']:
+            return False, "client_id must use https or http scheme"
+
+        # 2. For HTTP, only allow localhost/loopback
+        if parsed.scheme == 'http':
+            if parsed.hostname not in ['localhost', '127.0.0.1', '[::1]', '::1']:
+                return False, "HTTP scheme only allowed for localhost/127.0.0.1/[::1]"
+
+        # 3. Must have path component
+        if not parsed.path:
+            return False, "client_id must contain a path component"
+
+        # 4. Check for . or .. segments
+        path_segments = parsed.path.split('/')
+        if '.' in path_segments or '..' in path_segments:
+            return False, "client_id must not contain . or .. path segments"
+
+        # 5. Must NOT have fragment
+        if parsed.fragment:
+            return False, "client_id must not contain a fragment"
+
+        # 6. Must NOT have user info
+        if parsed.username or parsed.password:
+            return False, "client_id must not contain username or password"
+
+        # 7. Check hostname (no raw IPs except loopback)
+        if parsed.hostname:
+            # Check if it's an IP address
+            import ipaddress
+            try:
+                ip = ipaddress.ip_address(parsed.hostname)
+                # Only allow loopback IPs
+                if not ip.is_loopback:
+                    return False, f"client_id must not use IP address {parsed.hostname}"
+            except ValueError:
+                # Not an IP, that's good (it's a domain)
+                pass
+
+        return True, ""
+
+    except Exception as e:
+        return False, f"Invalid URL: {e}"
+```
+
+### Priority 2: Update normalize_client_id()
+
+Should call validate_client_id() first, then normalize.
+
+### Priority 3: Add comprehensive tests
+
+Test all validation rules with both positive and negative cases.
+
+## Conclusion
+
+**Current Status: NOT COMPLIANT** ❌
+
+Gondulf's client_id validation is currently **not compliant** with W3C IndieAuth specification Section 3.2. We have several missing validations that need to be implemented before v1.0.0 release.
+
+### Required Actions Before v1.0.0
+
+1. Implement complete `validate_client_id()` function
+2. Update `normalize_client_id()` to validate first
+3. Add support for HTTP on localhost/127.0.0.1/[::1]
+4. Add tests for all validation rules
+5. Update authorization endpoint to use new validation
+
+This is a **BLOCKING ISSUE** for v1.0.0 release as it affects spec compliance.

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "gondulf"
-version = "1.0.1"
+version = "1.0.2"
 description = "A self-hosted IndieAuth server implementation"
 readme = "README.md"
 requires-python = ">=3.10"

src/gondulf/templates/verification_error.html

@@ -33,7 +33,7 @@
 {% endif %}
 
 <p>
-    <a href="/authorize?client_id={{ client_id }}&redirect_uri={{ redirect_uri }}&response_type={{ response_type }}&state={{ state }}&code_challenge={{ code_challenge }}&code_challenge_method={{ code_challenge_method }}&scope={{ scope }}&me={{ me }}">
+    <a href="/authorize?client_id={{ client_id }}&redirect_uri={{ redirect_uri }}&response_type={{ response_type }}&state={{ state }}{% if code_challenge %}&code_challenge={{ code_challenge }}&code_challenge_method={{ code_challenge_method }}{% endif %}&scope={{ scope }}&me={{ me }}">
         Try Again
     </a>
 </p>

src/gondulf/templates/verify_code.html

@@ -36,7 +36,7 @@
 
 <p class="help-text">
     Did not receive a code? Check your spam folder.
-    <a href="/authorize?client_id={{ client_id }}&redirect_uri={{ redirect_uri }}&response_type={{ response_type }}&state={{ state }}&code_challenge={{ code_challenge }}&code_challenge_method={{ code_challenge_method }}&scope={{ scope }}&me={{ me }}">
+    <a href="/authorize?client_id={{ client_id }}&redirect_uri={{ redirect_uri }}&response_type={{ response_type }}&state={{ state }}{% if code_challenge %}&code_challenge={{ code_challenge }}&code_challenge_method={{ code_challenge_method }}{% endif %}&scope={{ scope }}&me={{ me }}">
         Request a new code
     </a>
 </p>

test_client_id_spec_compliance.py

@@ -0,0 +1,93 @@
+#!/usr/bin/env python3
+"""Test script to verify client_id validation compliance with W3C IndieAuth spec Section 3.2."""
+
+import sys
+from urllib.parse import urlparse
+
+# Import the validation function
+sys.path.insert(0, '/home/phil/Projects/Gondulf/src')
+from gondulf.utils.validation import normalize_client_id
+
+
+def test_client_id(client_id: str, should_pass: bool, reason: str):
+    """Test a single client_id against the spec."""
+    try:
+        result = normalize_client_id(client_id)
+        passed = True
+        print(f"✓ Accepted: {client_id} -> {result}")
+    except ValueError as e:
+        passed = False
+        print(f"✗ Rejected: {client_id} ({str(e)})")
+
+    if passed != should_pass:
+        print(f"  ERROR: Expected {'pass' if should_pass else 'fail'}: {reason}")
+        return False
+    return True
+
+
+def main():
+    """Run comprehensive client_id validation tests."""
+    print("Testing client_id validation against W3C IndieAuth spec Section 3.2")
+    print("=" * 70)
+
+    tests = [
+        # Valid client_ids per spec
+        ("https://example.com", True, "Valid HTTPS URL with domain"),
+        ("https://example.com/", True, "Valid HTTPS URL with path /"),
+        ("https://example.com/app", True, "Valid HTTPS URL with path"),
+        ("https://example.com/app/client", True, "Valid HTTPS URL with multiple path segments"),
+        ("https://example.com?foo=bar", True, "Valid HTTPS URL with query string"),
+        ("https://example.com:8080", True, "Valid HTTPS URL with port"),
+        ("https://example.com:8080/app", True, "Valid HTTPS URL with port and path"),
+        ("http://localhost", True, "Valid HTTP localhost (loopback allowed)"),
+        ("http://localhost:3000", True, "Valid HTTP localhost with port"),
+        ("http://127.0.0.1", True, "Valid HTTP IPv4 loopback"),
+        ("http://127.0.0.1:3000", True, "Valid HTTP IPv4 loopback with port"),
+        ("http://[::1]", True, "Valid HTTP IPv6 loopback"),
+        ("http://[::1]:3000", True, "Valid HTTP IPv6 loopback with port"),
+
+        # Invalid client_ids per spec
+        ("http://example.com", False, "HTTP not allowed for non-loopback"),
+        ("https://example.com#fragment", False, "Fragment not allowed"),
+        ("https://user:pass@example.com", False, "Username/password not allowed"),
+        ("https://192.168.1.1", False, "IPv4 address not allowed (except 127.0.0.1)"),
+        ("https://[2001:db8::1]", False, "IPv6 address not allowed (except ::1)"),
+        ("https://10.0.0.1", False, "Private IPv4 not allowed"),
+        ("https://172.16.0.1", False, "Private IPv4 not allowed"),
+        ("example.com", False, "Missing scheme"),
+        ("ftp://example.com", False, "Invalid scheme (not http/https)"),
+        ("https://", False, "Missing hostname"),
+        ("https://example.com/../secret", False, "Path with .. segments not allowed"),
+        ("https://example.com/./secret", False, "Path with . segments not allowed"),
+
+        # Edge cases that spec might not explicitly cover
+        ("HTTPS://EXAMPLE.COM", True, "Uppercase scheme/host should be normalized"),
+        ("https://example.com:443", True, "Default HTTPS port should be normalized"),
+        ("http://localhost:80", True, "Default HTTP port on localhost"),
+        ("https://xn--e1afmkfd.xn--p1ai", True, "Internationalized domain names (IDN)"),
+    ]
+
+    all_passed = True
+    pass_count = 0
+    fail_count = 0
+
+    for client_id, should_pass, reason in tests:
+        if test_client_id(client_id, should_pass, reason):
+            pass_count += 1
+        else:
+            fail_count += 1
+            all_passed = False
+
+    print("\n" + "=" * 70)
+    print(f"Results: {pass_count} passed, {fail_count} failed")
+
+    if all_passed:
+        print("\n✅ All tests passed! client_id validation appears spec-compliant.")
+    else:
+        print("\n❌ Some tests failed. Review the validation logic against the spec.")
+
+    return 0 if all_passed else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())

uv.lock

@@ -431,7 +431,7 @@ wheels = [
 
 [[package]]
 name = "gondulf"
-version = "1.0.0rc1"
+version = "1.0.1"
 source = { editable = "." }
 dependencies = [
     { name = "aiosmtplib" },