From 20b8392821ad54a34c9087791770c0f99f76e8e1 Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Thu, 25 Sep 2025 10:38:54 -0400 Subject: [PATCH 1/6] Refactor --- internal/api/handlers/v0/auth/common.go | 204 ++++++++++++++++++++++++ internal/api/handlers/v0/auth/dns.go | 149 +++-------------- internal/api/handlers/v0/auth/http.go | 114 +++---------- 3 files changed, 242 insertions(+), 225 deletions(-) create mode 100644 internal/api/handlers/v0/auth/common.go diff --git a/internal/api/handlers/v0/auth/common.go b/internal/api/handlers/v0/auth/common.go new file mode 100644 index 000000000..434a4c893 --- /dev/null +++ b/internal/api/handlers/v0/auth/common.go @@ -0,0 +1,204 @@ +package auth + +import ( + "context" + "crypto/ed25519" + "encoding/base64" + "encoding/hex" + "fmt" + "regexp" + "strings" + "time" + + "github.com/modelcontextprotocol/registry/internal/auth" + "github.com/modelcontextprotocol/registry/internal/config" +) + +// CoreTokenExchangeInput represents the common input structure for token exchange +type CoreTokenExchangeInput struct { + Domain string `json:"domain" doc:"Domain name" example:"example.com" required:"true"` + Timestamp string `json:"timestamp" doc:"RFC3339 timestamp" example:"2023-01-01T00:00:00Z" required:"true"` + SignedTimestamp string `json:"signed_timestamp" doc:"Hex-encoded Ed25519 signature of timestamp" example:"abcdef1234567890" required:"true"` +} + +// CoreAuthHandler represents the common handler structure +type CoreAuthHandler struct { + config *config.Config + jwtManager *auth.JWTManager +} + +// NewCoreAuthHandler creates a new core authentication handler +func NewCoreAuthHandler(cfg *config.Config) *CoreAuthHandler { + return &CoreAuthHandler{ + config: cfg, + jwtManager: auth.NewJWTManager(cfg), + } +} + +// ValidateDomainAndTimestamp validates the domain format and timestamp +func ValidateDomainAndTimestamp(domain, timestamp string) (*time.Time, error) { + // Validate domain format + if !IsValidDomain(domain) { + return nil, fmt.Errorf("invalid domain format") + } + + // Parse and validate timestamp + ts, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return nil, fmt.Errorf("invalid timestamp format: %w", err) + } + + // Check timestamp is within 15 seconds + now := time.Now() + if ts.Before(now.Add(-15*time.Second)) || ts.After(now.Add(15*time.Second)) { + return nil, fmt.Errorf("timestamp outside valid window (±15 seconds)") + } + + return &ts, nil +} + +// DecodeAndValidateSignature decodes and validates the signature format +func DecodeAndValidateSignature(signedTimestamp string) ([]byte, error) { + // Decode signature + signature, err := hex.DecodeString(signedTimestamp) + if err != nil { + return nil, fmt.Errorf("invalid signature format, must be hex: %w", err) + } + + if len(signature) != ed25519.SignatureSize { + return nil, fmt.Errorf("invalid signature length: expected %d, got %d", ed25519.SignatureSize, len(signature)) + } + + return signature, nil +} + +// VerifySignatureWithKeys verifies signature against a list of public keys +func VerifySignatureWithKeys(publicKeys []ed25519.PublicKey, messageBytes []byte, signature []byte) bool { + for _, publicKey := range publicKeys { + if ed25519.Verify(publicKey, messageBytes, signature) { + return true + } + } + return false +} + +// VerifySignatureWithKey verifies signature against a single public key +func VerifySignatureWithKey(publicKey ed25519.PublicKey, messageBytes []byte, signature []byte) bool { + return ed25519.Verify(publicKey, messageBytes, signature) +} + +// BuildPermissions builds permissions for a domain with optional subdomain support +func BuildPermissions(domain string, includeSubdomains bool) []auth.Permission { + reverseDomain := ReverseString(domain) + + permissions := []auth.Permission{ + // Grant permissions for the exact domain (e.g., com.example/*) + { + Action: auth.PermissionActionPublish, + ResourcePattern: fmt.Sprintf("%s/*", reverseDomain), + }, + } + + if includeSubdomains { + // DNS implies a hierarchy where subdomains are treated as part of the parent domain, + // therefore we grant permissions for all subdomains (e.g., com.example.*) + // This is in line with other DNS-based authentication methods e.g. ACME DNS-01 challenges + permissions = append(permissions, auth.Permission{ + Action: auth.PermissionActionPublish, + ResourcePattern: fmt.Sprintf("%s.*", reverseDomain), + }) + } + + return permissions +} + +// CreateJWTClaimsAndToken creates JWT claims and generates a token response +func (h *CoreAuthHandler) CreateJWTClaimsAndToken(ctx context.Context, authMethod auth.Method, domain string, permissions []auth.Permission) (*auth.TokenResponse, error) { + // Create JWT claims + jwtClaims := auth.JWTClaims{ + AuthMethod: authMethod, + AuthMethodSubject: domain, + Permissions: permissions, + } + + // Generate Registry JWT token + tokenResponse, err := h.jwtManager.GenerateTokenResponse(ctx, jwtClaims) + if err != nil { + return nil, fmt.Errorf("failed to generate JWT token: %w", err) + } + + return tokenResponse, nil +} + +// ParseMCPKeyFromString parses an Ed25519 public key from MCP format string +func ParseMCPKeyFromString(input string) (ed25519.PublicKey, error) { + // Expected format: v=MCPv1; k=ed25519; p= + mcpPattern := GetMCPKeyPattern() + + matches := mcpPattern.FindStringSubmatch(input) + if len(matches) != 2 { + return nil, fmt.Errorf("invalid key format, expected: v=MCPv1; k=ed25519; p=") + } + + // Decode base64 public key + publicKeyBytes, err := base64.StdEncoding.DecodeString(matches[1]) + if err != nil { + return nil, fmt.Errorf("failed to decode base64 public key: %w", err) + } + + if len(publicKeyBytes) != ed25519.PublicKeySize { + return nil, fmt.Errorf("invalid public key length: expected %d, got %d", ed25519.PublicKeySize, len(publicKeyBytes)) + } + + return ed25519.PublicKey(publicKeyBytes), nil +} + +// ParseMCPKeysFromStrings parses multiple Ed25519 public keys from MCP format strings +func ParseMCPKeysFromStrings(inputs []string) []ed25519.PublicKey { + var publicKeys []ed25519.PublicKey + mcpPattern := GetMCPKeyPattern() + + for _, input := range inputs { + matches := mcpPattern.FindStringSubmatch(input) + if len(matches) == 2 { + // Decode base64 public key + publicKeyBytes, err := base64.StdEncoding.DecodeString(matches[1]) + if err != nil { + continue // Skip invalid keys + } + + if len(publicKeyBytes) != ed25519.PublicKeySize { + continue // Skip invalid key sizes + } + + publicKeys = append(publicKeys, ed25519.PublicKey(publicKeyBytes)) + } + } + + return publicKeys +} + +// GetMCPKeyPattern returns the compiled regex pattern for MCP key format +func GetMCPKeyPattern() *regexp.Regexp { + return regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) +} + +// IsValidDomain validates domain format +func IsValidDomain(domain string) bool { + if len(domain) == 0 || len(domain) > 253 { + return false + } + + // Check for valid characters and structure + domainPattern := regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) + return domainPattern.MatchString(domain) +} + +// ReverseString reverses a domain string (example.com -> com.example) +func ReverseString(domain string) string { + parts := strings.Split(domain, ".") + for i, j := 0, len(parts)-1; i < j; i, j = i+1, j-1 { + parts[i], parts[j] = parts[j], parts[i] + } + return strings.Join(parts, ".") +} \ No newline at end of file diff --git a/internal/api/handlers/v0/auth/dns.go b/internal/api/handlers/v0/auth/dns.go index 991f5132e..cc09355f7 100644 --- a/internal/api/handlers/v0/auth/dns.go +++ b/internal/api/handlers/v0/auth/dns.go @@ -2,15 +2,9 @@ package auth import ( "context" - "crypto/ed25519" - "encoding/base64" - "encoding/hex" "fmt" "net" "net/http" - "regexp" - "strings" - "time" "github.com/danielgtaylor/huma/v2" v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0" @@ -20,11 +14,7 @@ import ( // DNSTokenExchangeInput represents the input for DNS-based authentication type DNSTokenExchangeInput struct { - Body struct { - Domain string `json:"domain" doc:"Domain name" example:"example.com" required:"true"` - Timestamp string `json:"timestamp" doc:"RFC3339 timestamp" example:"2023-01-01T00:00:00Z" required:"true"` - SignedTimestamp string `json:"signed_timestamp" doc:"Hex-encoded Ed25519 signature of timestamp" example:"abcdef1234567890" required:"true"` - } + Body CoreTokenExchangeInput } // DNSResolver defines the interface for DNS resolution @@ -42,17 +32,15 @@ func (r *DefaultDNSResolver) LookupTXT(ctx context.Context, name string) ([]stri // DNSAuthHandler handles DNS-based authentication type DNSAuthHandler struct { - config *config.Config - jwtManager *auth.JWTManager - resolver DNSResolver + CoreAuthHandler + resolver DNSResolver } // NewDNSAuthHandler creates a new DNS authentication handler func NewDNSAuthHandler(cfg *config.Config) *DNSAuthHandler { return &DNSAuthHandler{ - config: cfg, - jwtManager: auth.NewJWTManager(cfg), - resolver: &DefaultDNSResolver{}, + CoreAuthHandler: *NewCoreAuthHandler(cfg), + resolver: &DefaultDNSResolver{}, } } @@ -87,31 +75,16 @@ func RegisterDNSEndpoint(api huma.API, cfg *config.Config) { // ExchangeToken exchanges DNS signature for a Registry JWT token func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { - // Validate domain format - if !isValidDomain(domain) { - return nil, fmt.Errorf("invalid domain format") - } - - // Parse and validate timestamp - ts, err := time.Parse(time.RFC3339, timestamp) + // Validate domain and timestamp using shared utility + _, err := ValidateDomainAndTimestamp(domain, timestamp) if err != nil { - return nil, fmt.Errorf("invalid timestamp format: %w", err) - } - - // Check timestamp is within 15 seconds - now := time.Now() - if ts.Before(now.Add(-15*time.Second)) || ts.After(now.Add(15*time.Second)) { - return nil, fmt.Errorf("timestamp outside valid window (±15 seconds)") + return nil, err } - // Decode signature - signature, err := hex.DecodeString(signedTimestamp) + // Decode and validate signature using shared utility + signature, err := DecodeAndValidateSignature(signedTimestamp) if err != nil { - return nil, fmt.Errorf("invalid signature format, must be hex: %w", err) - } - - if len(signature) != ed25519.SignatureSize { - return nil, fmt.Errorf("invalid signature length: expected %d, got %d", ed25519.SignatureSize, len(signature)) + return nil, err } // Lookup DNS TXT records @@ -120,108 +93,24 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err) } - // Parse public keys from TXT records - publicKeys := h.parsePublicKeysFromTXT(txtRecords) + // Parse public keys from TXT records using shared utility + publicKeys := ParseMCPKeysFromStrings(txtRecords) if len(publicKeys) == 0 { return nil, fmt.Errorf("no valid MCP public keys found in DNS TXT records") } - // Verify signature with any of the public keys + // Verify signature with any of the public keys using shared utility messageBytes := []byte(timestamp) - signatureValid := false - for _, publicKey := range publicKeys { - if ed25519.Verify(publicKey, messageBytes, signature) { - signatureValid = true - break - } - } - - if !signatureValid { + if !VerifySignatureWithKeys(publicKeys, messageBytes, signature) { return nil, fmt.Errorf("signature verification failed") } - // Build permissions for domain and subdomains - permissions := h.buildPermissions(domain) - - // Create JWT claims - jwtClaims := auth.JWTClaims{ - AuthMethod: auth.MethodDNS, - AuthMethodSubject: domain, - Permissions: permissions, - } - - // Generate Registry JWT token - tokenResponse, err := h.jwtManager.GenerateTokenResponse(ctx, jwtClaims) - if err != nil { - return nil, fmt.Errorf("failed to generate JWT token: %w", err) - } + // Build permissions for domain and subdomains using shared utility (DNS includes subdomains) + permissions := BuildPermissions(domain, true) - return tokenResponse, nil + // Create JWT claims and token using shared utility + return h.CreateJWTClaimsAndToken(ctx, auth.MethodDNS, domain, permissions) } -// parsePublicKeysFromTXT parses Ed25519 public keys from DNS TXT records -func (h *DNSAuthHandler) parsePublicKeysFromTXT(txtRecords []string) []ed25519.PublicKey { - var publicKeys []ed25519.PublicKey - mcpPattern := regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) - - for _, record := range txtRecords { - matches := mcpPattern.FindStringSubmatch(record) - if len(matches) == 2 { - // Decode base64 public key - publicKeyBytes, err := base64.StdEncoding.DecodeString(matches[1]) - if err != nil { - continue // Skip invalid keys - } - - if len(publicKeyBytes) != ed25519.PublicKeySize { - continue // Skip invalid key sizes - } - - publicKeys = append(publicKeys, ed25519.PublicKey(publicKeyBytes)) - } - } - - return publicKeys -} - -// buildPermissions builds permissions for a domain and its subdomains using reverse DNS notation -func (h *DNSAuthHandler) buildPermissions(domain string) []auth.Permission { - reverseDomain := reverseString(domain) - - permissions := []auth.Permission{ - // Grant permissions for the exact domain (e.g., com.example/*) - { - Action: auth.PermissionActionPublish, - ResourcePattern: fmt.Sprintf("%s/*", reverseDomain), - }, - // DNS implies a hierarchy where subdomains are treated as part of the parent domain, - // therefore we grant permissions for all subdomains (e.g., com.example.*) - // This is in line with other DNS-based authentication methods e.g. ACME DNS-01 challenges - { - Action: auth.PermissionActionPublish, - ResourcePattern: fmt.Sprintf("%s.*", reverseDomain), - }, - } - - return permissions -} - -// reverseString reverses a domain string (example.com -> com.example) -func reverseString(domain string) string { - parts := strings.Split(domain, ".") - for i, j := 0, len(parts)-1; i < j; i, j = i+1, j-1 { - parts[i], parts[j] = parts[j], parts[i] - } - return strings.Join(parts, ".") -} -func isValidDomain(domain string) bool { - if len(domain) == 0 || len(domain) > 253 { - return false - } - - // Check for valid characters and structure - domainPattern := regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) - return domainPattern.MatchString(domain) -} diff --git a/internal/api/handlers/v0/auth/http.go b/internal/api/handlers/v0/auth/http.go index bf05dc5aa..1281b52c1 100644 --- a/internal/api/handlers/v0/auth/http.go +++ b/internal/api/handlers/v0/auth/http.go @@ -2,13 +2,9 @@ package auth import ( "context" - "crypto/ed25519" - "encoding/base64" - "encoding/hex" "fmt" "io" "net/http" - "regexp" "strings" "time" @@ -20,11 +16,7 @@ import ( // HTTPTokenExchangeInput represents the input for HTTP-based authentication type HTTPTokenExchangeInput struct { - Body struct { - Domain string `json:"domain" doc:"Domain name" example:"example.com" required:"true"` - Timestamp string `json:"timestamp" doc:"RFC3339 timestamp" example:"2023-01-01T00:00:00Z" required:"true"` - SignedTimestamp string `json:"signed_timestamp" doc:"Hex-encoded Ed25519 signature of timestamp" example:"abcdef1234567890" required:"true"` - } + Body CoreTokenExchangeInput } // HTTPKeyFetcher defines the interface for fetching HTTP keys @@ -86,17 +78,15 @@ func (f *DefaultHTTPKeyFetcher) FetchKey(ctx context.Context, domain string) (st // HTTPAuthHandler handles HTTP-based authentication type HTTPAuthHandler struct { - config *config.Config - jwtManager *auth.JWTManager - fetcher HTTPKeyFetcher + CoreAuthHandler + fetcher HTTPKeyFetcher } // NewHTTPAuthHandler creates a new HTTP authentication handler func NewHTTPAuthHandler(cfg *config.Config) *HTTPAuthHandler { return &HTTPAuthHandler{ - config: cfg, - jwtManager: auth.NewJWTManager(cfg), - fetcher: NewDefaultHTTPKeyFetcher(), + CoreAuthHandler: *NewCoreAuthHandler(cfg), + fetcher: NewDefaultHTTPKeyFetcher(), } } @@ -131,31 +121,16 @@ func RegisterHTTPEndpoint(api huma.API, cfg *config.Config) { // ExchangeToken exchanges HTTP signature for a Registry JWT token func (h *HTTPAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { - // Validate domain format - if !isValidDomain(domain) { - return nil, fmt.Errorf("invalid domain format") - } - - // Parse and validate timestamp - ts, err := time.Parse(time.RFC3339, timestamp) + // Validate domain and timestamp using shared utility + _, err := ValidateDomainAndTimestamp(domain, timestamp) if err != nil { - return nil, fmt.Errorf("invalid timestamp format: %w", err) + return nil, err } - // Check timestamp is within 15 seconds - now := time.Now() - if ts.Before(now.Add(-15*time.Second)) || ts.After(now.Add(15*time.Second)) { - return nil, fmt.Errorf("timestamp outside valid window (±15 seconds)") - } - - // Decode signature - signature, err := hex.DecodeString(signedTimestamp) + // Decode and validate signature using shared utility + signature, err := DecodeAndValidateSignature(signedTimestamp) if err != nil { - return nil, fmt.Errorf("invalid signature format, must be hex: %w", err) - } - - if len(signature) != ed25519.SignatureSize { - return nil, fmt.Errorf("invalid signature length: expected %d, got %d", ed25519.SignatureSize, len(signature)) + return nil, err } // Fetch public key from HTTP endpoint @@ -164,74 +139,23 @@ func (h *HTTPAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, return nil, fmt.Errorf("failed to fetch public key: %w", err) } - // Parse public key from HTTP response - publicKey, err := h.parsePublicKeyFromHTTP(keyResponse) + // Parse public key from HTTP response using shared utility + publicKey, err := ParseMCPKeyFromString(keyResponse) if err != nil { return nil, fmt.Errorf("failed to parse public key: %w", err) } - // Verify signature + // Verify signature using shared utility messageBytes := []byte(timestamp) - if !ed25519.Verify(publicKey, messageBytes, signature) { + if !VerifySignatureWithKey(publicKey, messageBytes, signature) { return nil, fmt.Errorf("signature verification failed") } - // Build permissions for domain and subdomains - permissions := h.buildPermissions(domain) - - // Create JWT claims - jwtClaims := auth.JWTClaims{ - AuthMethod: auth.MethodHTTP, - AuthMethodSubject: domain, - Permissions: permissions, - } - - // Generate Registry JWT token - tokenResponse, err := h.jwtManager.GenerateTokenResponse(ctx, jwtClaims) - if err != nil { - return nil, fmt.Errorf("failed to generate JWT token: %w", err) - } - - return tokenResponse, nil -} - -// parsePublicKeyFromHTTP parses Ed25519 public key from HTTP response -func (h *HTTPAuthHandler) parsePublicKeyFromHTTP(response string) (ed25519.PublicKey, error) { - // Expected format: v=MCPv1; k=ed25519; p= - mcpPattern := regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) - - matches := mcpPattern.FindStringSubmatch(response) - if len(matches) != 2 { - return nil, fmt.Errorf("invalid key format, expected: v=MCPv1; k=ed25519; p=") - } - - // Decode base64 public key - publicKeyBytes, err := base64.StdEncoding.DecodeString(matches[1]) - if err != nil { - return nil, fmt.Errorf("failed to decode base64 public key: %w", err) - } + // Build permissions for domain (HTTP does not include subdomains) + permissions := BuildPermissions(domain, false) - if len(publicKeyBytes) != ed25519.PublicKeySize { - return nil, fmt.Errorf("invalid public key length: expected %d, got %d", ed25519.PublicKeySize, len(publicKeyBytes)) - } - - return ed25519.PublicKey(publicKeyBytes), nil + // Create JWT claims and token using shared utility + return h.CreateJWTClaimsAndToken(ctx, auth.MethodHTTP, domain, permissions) } -// buildPermissions builds permissions for a domain and its subdomains using reverse DNS notation -func (h *HTTPAuthHandler) buildPermissions(domain string) []auth.Permission { - reverseDomain := reverseString(domain) - - permissions := []auth.Permission{ - // Grant permissions for the exact domain (e.g., com.example/*) - { - Action: auth.PermissionActionPublish, - ResourcePattern: fmt.Sprintf("%s/*", reverseDomain), - }, - // HTTP does not imply a hierarchy of ownership of subdomains, unlike DNS - // Therefore this does not give permissions for subdomains - // This is consistent with similar protocols, e.g. ACME HTTP-01 - } - return permissions -} From ecfb4c1aac8d835ffe7ed5bdf57ea09aa878b920 Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Thu, 25 Sep 2025 11:27:59 -0400 Subject: [PATCH 2/6] Polish --- internal/api/handlers/v0/auth/common.go | 58 ++++------------------ internal/api/handlers/v0/auth/dns.go | 9 +--- internal/api/handlers/v0/auth/http.go | 17 ++----- internal/api/handlers/v0/auth/http_test.go | 6 +-- 4 files changed, 18 insertions(+), 72 deletions(-) diff --git a/internal/api/handlers/v0/auth/common.go b/internal/api/handlers/v0/auth/common.go index 434a4c893..3a05fb9cc 100644 --- a/internal/api/handlers/v0/auth/common.go +++ b/internal/api/handlers/v0/auth/common.go @@ -37,18 +37,16 @@ func NewCoreAuthHandler(cfg *config.Config) *CoreAuthHandler { // ValidateDomainAndTimestamp validates the domain format and timestamp func ValidateDomainAndTimestamp(domain, timestamp string) (*time.Time, error) { - // Validate domain format if !IsValidDomain(domain) { return nil, fmt.Errorf("invalid domain format") } - // Parse and validate timestamp ts, err := time.Parse(time.RFC3339, timestamp) if err != nil { return nil, fmt.Errorf("invalid timestamp format: %w", err) } - // Check timestamp is within 15 seconds + // Check timestamp is within 15 seconds, to allow for clock skew now := time.Now() if ts.Before(now.Add(-15*time.Second)) || ts.After(now.Add(15*time.Second)) { return nil, fmt.Errorf("timestamp outside valid window (±15 seconds)") @@ -57,9 +55,7 @@ func ValidateDomainAndTimestamp(domain, timestamp string) (*time.Time, error) { return &ts, nil } -// DecodeAndValidateSignature decodes and validates the signature format func DecodeAndValidateSignature(signedTimestamp string) ([]byte, error) { - // Decode signature signature, err := hex.DecodeString(signedTimestamp) if err != nil { return nil, fmt.Errorf("invalid signature format, must be hex: %w", err) @@ -72,7 +68,6 @@ func DecodeAndValidateSignature(signedTimestamp string) ([]byte, error) { return signature, nil } -// VerifySignatureWithKeys verifies signature against a list of public keys func VerifySignatureWithKeys(publicKeys []ed25519.PublicKey, messageBytes []byte, signature []byte) bool { for _, publicKey := range publicKeys { if ed25519.Verify(publicKey, messageBytes, signature) { @@ -82,11 +77,6 @@ func VerifySignatureWithKeys(publicKeys []ed25519.PublicKey, messageBytes []byte return false } -// VerifySignatureWithKey verifies signature against a single public key -func VerifySignatureWithKey(publicKey ed25519.PublicKey, messageBytes []byte, signature []byte) bool { - return ed25519.Verify(publicKey, messageBytes, signature) -} - // BuildPermissions builds permissions for a domain with optional subdomain support func BuildPermissions(domain string, includeSubdomains bool) []auth.Permission { reverseDomain := ReverseString(domain) @@ -130,33 +120,9 @@ func (h *CoreAuthHandler) CreateJWTClaimsAndToken(ctx context.Context, authMetho return tokenResponse, nil } -// ParseMCPKeyFromString parses an Ed25519 public key from MCP format string -func ParseMCPKeyFromString(input string) (ed25519.PublicKey, error) { - // Expected format: v=MCPv1; k=ed25519; p= - mcpPattern := GetMCPKeyPattern() - - matches := mcpPattern.FindStringSubmatch(input) - if len(matches) != 2 { - return nil, fmt.Errorf("invalid key format, expected: v=MCPv1; k=ed25519; p=") - } - - // Decode base64 public key - publicKeyBytes, err := base64.StdEncoding.DecodeString(matches[1]) - if err != nil { - return nil, fmt.Errorf("failed to decode base64 public key: %w", err) - } - - if len(publicKeyBytes) != ed25519.PublicKeySize { - return nil, fmt.Errorf("invalid public key length: expected %d, got %d", ed25519.PublicKeySize, len(publicKeyBytes)) - } - - return ed25519.PublicKey(publicKeyBytes), nil -} - -// ParseMCPKeysFromStrings parses multiple Ed25519 public keys from MCP format strings func ParseMCPKeysFromStrings(inputs []string) []ed25519.PublicKey { var publicKeys []ed25519.PublicKey - mcpPattern := GetMCPKeyPattern() + mcpPattern := regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) for _, input := range inputs { matches := mcpPattern.FindStringSubmatch(input) @@ -178,12 +144,15 @@ func ParseMCPKeysFromStrings(inputs []string) []ed25519.PublicKey { return publicKeys } -// GetMCPKeyPattern returns the compiled regex pattern for MCP key format -func GetMCPKeyPattern() *regexp.Regexp { - return regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) +// ReverseString reverses a domain string (example.com -> com.example) +func ReverseString(domain string) string { + parts := strings.Split(domain, ".") + for i, j := 0, len(parts)-1; i < j; i, j = i+1, j-1 { + parts[i], parts[j] = parts[j], parts[i] + } + return strings.Join(parts, ".") } -// IsValidDomain validates domain format func IsValidDomain(domain string) bool { if len(domain) == 0 || len(domain) > 253 { return false @@ -193,12 +162,3 @@ func IsValidDomain(domain string) bool { domainPattern := regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) return domainPattern.MatchString(domain) } - -// ReverseString reverses a domain string (example.com -> com.example) -func ReverseString(domain string) string { - parts := strings.Split(domain, ".") - for i, j := 0, len(parts)-1; i < j; i, j = i+1, j-1 { - parts[i], parts[j] = parts[j], parts[i] - } - return strings.Join(parts, ".") -} \ No newline at end of file diff --git a/internal/api/handlers/v0/auth/dns.go b/internal/api/handlers/v0/auth/dns.go index cc09355f7..cce36fb40 100644 --- a/internal/api/handlers/v0/auth/dns.go +++ b/internal/api/handlers/v0/auth/dns.go @@ -75,13 +75,11 @@ func RegisterDNSEndpoint(api huma.API, cfg *config.Config) { // ExchangeToken exchanges DNS signature for a Registry JWT token func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { - // Validate domain and timestamp using shared utility _, err := ValidateDomainAndTimestamp(domain, timestamp) if err != nil { return nil, err } - // Decode and validate signature using shared utility signature, err := DecodeAndValidateSignature(signedTimestamp) if err != nil { return nil, err @@ -93,24 +91,19 @@ func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, s return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err) } - // Parse public keys from TXT records using shared utility publicKeys := ParseMCPKeysFromStrings(txtRecords) if len(publicKeys) == 0 { return nil, fmt.Errorf("no valid MCP public keys found in DNS TXT records") } - // Verify signature with any of the public keys using shared utility messageBytes := []byte(timestamp) if !VerifySignatureWithKeys(publicKeys, messageBytes, signature) { return nil, fmt.Errorf("signature verification failed") } - // Build permissions for domain and subdomains using shared utility (DNS includes subdomains) + // Build permissions for domain (DNS includes subdomains) permissions := BuildPermissions(domain, true) - // Create JWT claims and token using shared utility return h.CreateJWTClaimsAndToken(ctx, auth.MethodDNS, domain, permissions) } - - diff --git a/internal/api/handlers/v0/auth/http.go b/internal/api/handlers/v0/auth/http.go index 1281b52c1..0ee91dfee 100644 --- a/internal/api/handlers/v0/auth/http.go +++ b/internal/api/handlers/v0/auth/http.go @@ -121,41 +121,34 @@ func RegisterHTTPEndpoint(api huma.API, cfg *config.Config) { // ExchangeToken exchanges HTTP signature for a Registry JWT token func (h *HTTPAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { - // Validate domain and timestamp using shared utility _, err := ValidateDomainAndTimestamp(domain, timestamp) if err != nil { return nil, err } - // Decode and validate signature using shared utility signature, err := DecodeAndValidateSignature(signedTimestamp) if err != nil { return nil, err } - // Fetch public key from HTTP endpoint + // Fetch the key from well known HTTP endpoint keyResponse, err := h.fetcher.FetchKey(ctx, domain) if err != nil { return nil, fmt.Errorf("failed to fetch public key: %w", err) } - // Parse public key from HTTP response using shared utility - publicKey, err := ParseMCPKeyFromString(keyResponse) - if err != nil { - return nil, fmt.Errorf("failed to parse public key: %w", err) + publicKeys := ParseMCPKeysFromStrings([]string{keyResponse}) + if len(publicKeys) == 0 { + return nil, fmt.Errorf("failed to parse public key") } - // Verify signature using shared utility messageBytes := []byte(timestamp) - if !VerifySignatureWithKey(publicKey, messageBytes, signature) { + if !VerifySignatureWithKeys(publicKeys, messageBytes, signature) { return nil, fmt.Errorf("signature verification failed") } // Build permissions for domain (HTTP does not include subdomains) permissions := BuildPermissions(domain, false) - // Create JWT claims and token using shared utility return h.CreateJWTClaimsAndToken(ctx, auth.MethodHTTP, domain, permissions) } - - diff --git a/internal/api/handlers/v0/auth/http_test.go b/internal/api/handlers/v0/auth/http_test.go index 5ea77a61e..1c46d35df 100644 --- a/internal/api/handlers/v0/auth/http_test.go +++ b/internal/api/handlers/v0/auth/http_test.go @@ -131,7 +131,7 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { m.err = nil }, expectError: true, - errorContains: "invalid key format", + errorContains: "failed to parse public key", }, { name: "invalid base64 key", @@ -142,7 +142,7 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { m.err = nil }, expectError: true, - errorContains: "failed to decode base64 public key", + errorContains: "failed to parse public key", }, { name: "wrong key size", @@ -155,7 +155,7 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { m.err = nil }, expectError: true, - errorContains: "invalid public key length", + errorContains: "failed to parse public key", }, { name: "signature verification failure", From 910fa320f35eba8036564b6cd7edf49cc466f9b1 Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Thu, 25 Sep 2025 11:37:29 -0400 Subject: [PATCH 3/6] Improve tests --- internal/api/handlers/v0/auth/dns_test.go | 264 ++++++++++++++++ internal/api/handlers/v0/auth/http_test.go | 351 +++++++++++++++++++++ 2 files changed, 615 insertions(+) diff --git a/internal/api/handlers/v0/auth/dns_test.go b/internal/api/handlers/v0/auth/dns_test.go index e9c10d52c..db7c6c10e 100644 --- a/internal/api/handlers/v0/auth/dns_test.go +++ b/internal/api/handlers/v0/auth/dns_test.go @@ -194,3 +194,267 @@ func TestDNSAuthHandler_ExchangeToken(t *testing.T) { }) } } + +func TestDNSAuthHandler_Permissions(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + handler := auth.NewDNSAuthHandler(cfg) + jwtManager := intauth.NewJWTManager(cfg) + + // Generate a test key pair + publicKey, privateKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) + + tests := []struct { + name string + domain string + expectedPatterns []string + unexpectedPatterns []string + }{ + { + name: "simple domain", + domain: "example.com", + expectedPatterns: []string{ + "com.example/*", // exact domain pattern + "com.example.*", // subdomain pattern (DNS includes subdomains) + }, + unexpectedPatterns: []string{ + "example.com/*", // should be reversed + "*.com.example", // wrong wildcard position + }, + }, + { + name: "subdomain", + domain: "api.example.com", + expectedPatterns: []string{ + "com.example.api/*", // exact subdomain pattern + "com.example.api.*", // subdomain pattern + }, + unexpectedPatterns: []string{ + "com.example/*", // parent domain should not be included + "api.example.com/*", // should be reversed + }, + }, + { + name: "multi-level subdomain", + domain: "v1.api.example.com", + expectedPatterns: []string{ + "com.example.api.v1/*", // exact pattern + "com.example.api.v1.*", // subdomain pattern + }, + unexpectedPatterns: []string{ + "com.example/*", // parent domain should not be included + "com.example.api/*", // intermediate domain should not be included + "v1.api.example.com/*", // should be reversed + }, + }, + { + name: "single part domain", + domain: "localhost", + expectedPatterns: []string{ + "localhost/*", // exact pattern (no reversal needed) + "localhost.*", // subdomain pattern + }, + unexpectedPatterns: []string{ + "*.localhost", // wrong wildcard position + }, + }, + { + name: "hyphenated domain", + domain: "my-app.example-site.com", + expectedPatterns: []string{ + "com.example-site.my-app/*", // exact pattern + "com.example-site.my-app.*", // subdomain pattern + }, + unexpectedPatterns: []string{ + "my-app.example-site.com/*", // should be reversed + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set up mock resolver + mockResolver := &MockDNSResolver{ + txtRecords: map[string][]string{ + tt.domain: { + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + }, + }, + } + handler.SetResolver(mockResolver) + + // Generate signature + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(privateKey, []byte(timestamp)) + signedTimestamp := hex.EncodeToString(signature) + + // Exchange token + result, err := handler.ExchangeToken(context.Background(), tt.domain, timestamp, signedTimestamp) + require.NoError(t, err) + require.NotNil(t, result) + + // Validate JWT token + claims, err := jwtManager.ValidateToken(context.Background(), result.RegistryToken) + require.NoError(t, err) + + // Verify claims structure + assert.Equal(t, intauth.MethodDNS, claims.AuthMethod) + assert.Equal(t, tt.domain, claims.AuthMethodSubject) + assert.Len(t, claims.Permissions, 2) // DNS always grants both exact and subdomain permissions + + // Extract permission patterns + patterns := make([]string, len(claims.Permissions)) + for i, perm := range claims.Permissions { + patterns[i] = perm.ResourcePattern + // All permissions should be for publish action + assert.Equal(t, intauth.PermissionActionPublish, perm.Action) + } + + // Check expected patterns are present + for _, expectedPattern := range tt.expectedPatterns { + assert.Contains(t, patterns, expectedPattern, "Expected pattern %s not found", expectedPattern) + } + + // Check unexpected patterns are not present + for _, unexpectedPattern := range tt.unexpectedPatterns { + assert.NotContains(t, patterns, unexpectedPattern, "Unexpected pattern %s found", unexpectedPattern) + } + + // Verify the permission patterns work correctly with the JWT manager's HasPermission method + for _, expectedPattern := range tt.expectedPatterns { + // Find the permission with this pattern + var foundPerm *intauth.Permission + for _, perm := range claims.Permissions { + if perm.ResourcePattern == expectedPattern { + foundPerm = &perm + break + } + } + require.NotNil(t, foundPerm, "Permission with pattern %s not found", expectedPattern) + + // Test various resource scenarios + if strings.HasSuffix(expectedPattern, "/*") { + // Exact domain permissions (e.g., "com.example/*") + basePattern := strings.TrimSuffix(expectedPattern, "/*") + testResource := basePattern + "/my-package" + assert.True(t, jwtManager.HasPermission(testResource, intauth.PermissionActionPublish, claims.Permissions), + "Should have permission for %s with pattern %s", testResource, expectedPattern) + } else if strings.HasSuffix(expectedPattern, ".*") { + // Subdomain permissions (e.g., "com.example.*") + basePattern := strings.TrimSuffix(expectedPattern, ".*") + testResource := basePattern + ".subdomain/my-package" + assert.True(t, jwtManager.HasPermission(testResource, intauth.PermissionActionPublish, claims.Permissions), + "Should have permission for %s with pattern %s", testResource, expectedPattern) + } + } + }) + } +} + +func TestDNSAuthHandler_PermissionValidation(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + handler := auth.NewDNSAuthHandler(cfg) + jwtManager := intauth.NewJWTManager(cfg) + + // Generate a test key pair + publicKey, privateKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) + domain := "example.com" + + // Set up mock resolver + mockResolver := &MockDNSResolver{ + txtRecords: map[string][]string{ + domain: { + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + }, + }, + } + handler.SetResolver(mockResolver) + + // Generate signature and exchange token + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(privateKey, []byte(timestamp)) + signedTimestamp := hex.EncodeToString(signature) + + result, err := handler.ExchangeToken(context.Background(), domain, timestamp, signedTimestamp) + require.NoError(t, err) + + claims, err := jwtManager.ValidateToken(context.Background(), result.RegistryToken) + require.NoError(t, err) + + // Test permission validation scenarios + testCases := []struct { + name string + resource string + action intauth.PermissionAction + shouldPass bool + }{ + { + name: "exact domain resource with publish action", + resource: "com.example/my-package", + action: intauth.PermissionActionPublish, + shouldPass: true, + }, + { + name: "subdomain resource with publish action", + resource: "com.example.api/my-package", + action: intauth.PermissionActionPublish, + shouldPass: true, + }, + { + name: "deep subdomain resource with publish action", + resource: "com.example.v1.api/my-package", + action: intauth.PermissionActionPublish, + shouldPass: true, + }, + { + name: "different domain should fail", + resource: "com.otherdomain/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + { + name: "partial domain match should fail", + resource: "com.example-other/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + { + name: "parent domain should fail", + resource: "com/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + { + name: "edit action should fail (not granted)", + resource: "com.example/my-package", + action: intauth.PermissionActionEdit, + shouldPass: false, + }, + { + name: "resource without package separator should fail", + resource: "com.example", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hasPermission := jwtManager.HasPermission(tc.resource, tc.action, claims.Permissions) + if tc.shouldPass { + assert.True(t, hasPermission, "Expected permission for resource %s with action %s", tc.resource, tc.action) + } else { + assert.False(t, hasPermission, "Expected no permission for resource %s with action %s", tc.resource, tc.action) + } + }) + } +} diff --git a/internal/api/handlers/v0/auth/http_test.go b/internal/api/handlers/v0/auth/http_test.go index 1c46d35df..f25619d64 100644 --- a/internal/api/handlers/v0/auth/http_test.go +++ b/internal/api/handlers/v0/auth/http_test.go @@ -241,3 +241,354 @@ func TestDefaultHTTPKeyFetcher_FetchKey(t *testing.T) { _, err := fetcher.FetchKey(context.Background(), "nonexistent-test-domain-12345.com") assert.Error(t, err) } + +func TestHTTPAuthHandler_Permissions(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + handler := auth.NewHTTPAuthHandler(cfg) + jwtManager := intauth.NewJWTManager(cfg) + + // Generate a test key pair + publicKey, privateKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) + + tests := []struct { + name string + domain string + expectedPatterns []string + unexpectedPatterns []string + }{ + { + name: "simple domain", + domain: "example.com", + expectedPatterns: []string{ + "com.example/*", // exact domain pattern only (HTTP does not include subdomains) + }, + unexpectedPatterns: []string{ + "com.example.*", // HTTP should not grant subdomain permissions + "example.com/*", // should be reversed + "*.com.example", // wrong wildcard position + }, + }, + { + name: "subdomain", + domain: "api.example.com", + expectedPatterns: []string{ + "com.example.api/*", // exact subdomain pattern only + }, + unexpectedPatterns: []string{ + "com.example.api.*", // HTTP should not grant subdomain permissions + "com.example/*", // parent domain should not be included + "api.example.com/*", // should be reversed + }, + }, + { + name: "multi-level subdomain", + domain: "v1.api.example.com", + expectedPatterns: []string{ + "com.example.api.v1/*", // exact pattern only + }, + unexpectedPatterns: []string{ + "com.example.api.v1.*", // HTTP should not grant subdomain permissions + "com.example/*", // parent domain should not be included + "com.example.api/*", // intermediate domain should not be included + "v1.api.example.com/*", // should be reversed + }, + }, + { + name: "single part domain", + domain: "localhost", + expectedPatterns: []string{ + "localhost/*", // exact pattern only (no reversal needed) + }, + unexpectedPatterns: []string{ + "localhost.*", // HTTP should not grant subdomain permissions + "*.localhost", // wrong wildcard position + }, + }, + { + name: "hyphenated domain", + domain: "my-app.example-site.com", + expectedPatterns: []string{ + "com.example-site.my-app/*", // exact pattern only + }, + unexpectedPatterns: []string{ + "com.example-site.my-app.*", // HTTP should not grant subdomain permissions + "my-app.example-site.com/*", // should be reversed + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set up mock fetcher + mockFetcher := &MockHTTPKeyFetcher{ + keyResponses: map[string]string{ + tt.domain: fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + }, + } + handler.SetFetcher(mockFetcher) + + // Generate signature + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(privateKey, []byte(timestamp)) + signedTimestamp := hex.EncodeToString(signature) + + // Exchange token + result, err := handler.ExchangeToken(context.Background(), tt.domain, timestamp, signedTimestamp) + require.NoError(t, err) + require.NotNil(t, result) + + // Validate JWT token + claims, err := jwtManager.ValidateToken(context.Background(), result.RegistryToken) + require.NoError(t, err) + + // Verify claims structure + assert.Equal(t, intauth.MethodHTTP, claims.AuthMethod) + assert.Equal(t, tt.domain, claims.AuthMethodSubject) + assert.Len(t, claims.Permissions, 1) // HTTP only grants exact domain permissions + + // Extract permission patterns + patterns := make([]string, len(claims.Permissions)) + for i, perm := range claims.Permissions { + patterns[i] = perm.ResourcePattern + // All permissions should be for publish action + assert.Equal(t, intauth.PermissionActionPublish, perm.Action) + } + + // Check expected patterns are present + for _, expectedPattern := range tt.expectedPatterns { + assert.Contains(t, patterns, expectedPattern, "Expected pattern %s not found", expectedPattern) + } + + // Check unexpected patterns are not present + for _, unexpectedPattern := range tt.unexpectedPatterns { + assert.NotContains(t, patterns, unexpectedPattern, "Unexpected pattern %s found", unexpectedPattern) + } + + // Verify the permission patterns work correctly with the JWT manager's HasPermission method + for _, expectedPattern := range tt.expectedPatterns { + // Find the permission with this pattern + var foundPerm *intauth.Permission + for _, perm := range claims.Permissions { + if perm.ResourcePattern == expectedPattern { + foundPerm = &perm + break + } + } + require.NotNil(t, foundPerm, "Permission with pattern %s not found", expectedPattern) + + // Test resource scenarios - only exact domain should work for HTTP + if strings.HasSuffix(expectedPattern, "/*") { + // Exact domain permissions (e.g., "com.example/*") + basePattern := strings.TrimSuffix(expectedPattern, "/*") + testResource := basePattern + "/my-package" + assert.True(t, jwtManager.HasPermission(testResource, intauth.PermissionActionPublish, claims.Permissions), + "Should have permission for %s with pattern %s", testResource, expectedPattern) + + // Test that subdomain resources are NOT allowed for HTTP + subdomainResource := basePattern + ".subdomain/my-package" + assert.False(t, jwtManager.HasPermission(subdomainResource, intauth.PermissionActionPublish, claims.Permissions), + "Should NOT have permission for subdomain %s with HTTP auth", subdomainResource) + } + } + }) + } +} + +func TestHTTPAuthHandler_PermissionValidation(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + handler := auth.NewHTTPAuthHandler(cfg) + jwtManager := intauth.NewJWTManager(cfg) + + // Generate a test key pair + publicKey, privateKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) + domain := "example.com" + + // Set up mock fetcher + mockFetcher := &MockHTTPKeyFetcher{ + keyResponses: map[string]string{ + domain: fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + }, + } + handler.SetFetcher(mockFetcher) + + // Generate signature and exchange token + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(privateKey, []byte(timestamp)) + signedTimestamp := hex.EncodeToString(signature) + + result, err := handler.ExchangeToken(context.Background(), domain, timestamp, signedTimestamp) + require.NoError(t, err) + + claims, err := jwtManager.ValidateToken(context.Background(), result.RegistryToken) + require.NoError(t, err) + + // Test permission validation scenarios + testCases := []struct { + name string + resource string + action intauth.PermissionAction + shouldPass bool + }{ + { + name: "exact domain resource with publish action", + resource: "com.example/my-package", + action: intauth.PermissionActionPublish, + shouldPass: true, + }, + { + name: "subdomain resource should fail for HTTP", + resource: "com.example.api/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, // HTTP does not grant subdomain permissions + }, + { + name: "deep subdomain resource should fail for HTTP", + resource: "com.example.v1.api/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, // HTTP does not grant subdomain permissions + }, + { + name: "different domain should fail", + resource: "com.otherdomain/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + { + name: "partial domain match should fail", + resource: "com.example-other/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + { + name: "parent domain should fail", + resource: "com/my-package", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + { + name: "edit action should fail (not granted)", + resource: "com.example/my-package", + action: intauth.PermissionActionEdit, + shouldPass: false, + }, + { + name: "resource without package separator should fail", + resource: "com.example", + action: intauth.PermissionActionPublish, + shouldPass: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + hasPermission := jwtManager.HasPermission(tc.resource, tc.action, claims.Permissions) + if tc.shouldPass { + assert.True(t, hasPermission, "Expected permission for resource %s with action %s", tc.resource, tc.action) + } else { + assert.False(t, hasPermission, "Expected no permission for resource %s with action %s", tc.resource, tc.action) + } + }) + } +} + +func TestHTTPvsDNS_PermissionDifferences(t *testing.T) { + cfg := &config.Config{ + JWTPrivateKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + } + httpHandler := auth.NewHTTPAuthHandler(cfg) + dnsHandler := auth.NewDNSAuthHandler(cfg) + jwtManager := intauth.NewJWTManager(cfg) + + // Generate a test key pair + publicKey, privateKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) + domain := "example.com" + + // Set up mocks + mockFetcher := &MockHTTPKeyFetcher{ + keyResponses: map[string]string{ + domain: fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + }, + } + httpHandler.SetFetcher(mockFetcher) + + mockResolver := &MockDNSResolver{ + txtRecords: map[string][]string{ + domain: { + fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + }, + }, + } + dnsHandler.SetResolver(mockResolver) + + // Generate tokens from both handlers + timestamp := time.Now().UTC().Format(time.RFC3339) + signature := ed25519.Sign(privateKey, []byte(timestamp)) + signedTimestamp := hex.EncodeToString(signature) + + httpResult, err := httpHandler.ExchangeToken(context.Background(), domain, timestamp, signedTimestamp) + require.NoError(t, err) + + dnsResult, err := dnsHandler.ExchangeToken(context.Background(), domain, timestamp, signedTimestamp) + require.NoError(t, err) + + // Validate both tokens + httpClaims, err := jwtManager.ValidateToken(context.Background(), httpResult.RegistryToken) + require.NoError(t, err) + + dnsClaims, err := jwtManager.ValidateToken(context.Background(), dnsResult.RegistryToken) + require.NoError(t, err) + + // Compare permission counts + assert.Len(t, httpClaims.Permissions, 1, "HTTP should grant 1 permission (exact domain only)") + assert.Len(t, dnsClaims.Permissions, 2, "DNS should grant 2 permissions (exact domain + subdomains)") + + // Test resources that should behave differently + testCases := []struct { + name string + resource string + httpAllowed bool + dnsAllowed bool + }{ + { + name: "exact domain resource", + resource: "com.example/my-package", + httpAllowed: true, + dnsAllowed: true, + }, + { + name: "subdomain resource", + resource: "com.example.api/my-package", + httpAllowed: false, // HTTP does not grant subdomain permissions + dnsAllowed: true, // DNS grants subdomain permissions + }, + { + name: "deep subdomain resource", + resource: "com.example.v1.api/my-package", + httpAllowed: false, // HTTP does not grant subdomain permissions + dnsAllowed: true, // DNS grants subdomain permissions + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + httpPermission := jwtManager.HasPermission(tc.resource, intauth.PermissionActionPublish, httpClaims.Permissions) + dnsPermission := jwtManager.HasPermission(tc.resource, intauth.PermissionActionPublish, dnsClaims.Permissions) + + assert.Equal(t, tc.httpAllowed, httpPermission, "HTTP permission mismatch for %s", tc.resource) + assert.Equal(t, tc.dnsAllowed, dnsPermission, "DNS permission mismatch for %s", tc.resource) + }) + } +} From 130d510a8bd6e2e665d54c384a4b5823a2aa4b15 Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Thu, 25 Sep 2025 13:04:59 -0400 Subject: [PATCH 4/6] Fix lint issue --- internal/api/handlers/v0/auth/dns_test.go | 24 ++++++++++---------- internal/api/handlers/v0/auth/http_test.go | 26 ++++++++++++---------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/internal/api/handlers/v0/auth/dns_test.go b/internal/api/handlers/v0/auth/dns_test.go index db7c6c10e..994cb9677 100644 --- a/internal/api/handlers/v0/auth/dns_test.go +++ b/internal/api/handlers/v0/auth/dns_test.go @@ -45,7 +45,7 @@ func TestDNSAuthHandler_ExchangeToken(t *testing.T) { publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) mockResolver := &MockDNSResolver{ txtRecords: map[string][]string{ - "example.com": { + testDomain: { fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), }, }, @@ -63,7 +63,7 @@ func TestDNSAuthHandler_ExchangeToken(t *testing.T) { }{ { name: "successful authentication", - domain: "example.com", + domain: testDomain, timestamp: time.Now().UTC().Format(time.RFC3339), setupMock: func(_ *MockDNSResolver) { // Mock is already set up with valid key @@ -72,14 +72,14 @@ func TestDNSAuthHandler_ExchangeToken(t *testing.T) { }, { name: "multiple keys", - domain: "example.com", + domain: testDomain, timestamp: time.Now().UTC().Format(time.RFC3339), setupMock: func(m *MockDNSResolver) { publicKey, _, err := ed25519.GenerateKey(nil) require.NoError(t, err) otherPublicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) - m.txtRecords["example.com"] = []string{ + m.txtRecords[testDomain] = []string{ fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", "someNonsense"), fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", otherPublicKeyB64), @@ -96,14 +96,14 @@ func TestDNSAuthHandler_ExchangeToken(t *testing.T) { }, { name: "timestamp too old", - domain: "example.com", + domain: testDomain, timestamp: time.Now().Add(-30 * time.Second).UTC().Format(time.RFC3339), expectError: true, errorContains: "timestamp outside valid window", }, { name: "timestamp too far in the future", - domain: "example.com", + domain: testDomain, timestamp: time.Now().Add(30 * time.Second).UTC().Format(time.RFC3339), expectError: true, errorContains: "timestamp outside valid window", @@ -216,14 +216,14 @@ func TestDNSAuthHandler_Permissions(t *testing.T) { }{ { name: "simple domain", - domain: "example.com", + domain: testDomain, expectedPatterns: []string{ "com.example/*", // exact domain pattern "com.example.*", // subdomain pattern (DNS includes subdomains) }, unexpectedPatterns: []string{ - "example.com/*", // should be reversed - "*.com.example", // wrong wildcard position + testDomain + "/*", // should be reversed + "*.com.example", // wrong wildcard position }, }, { @@ -234,8 +234,8 @@ func TestDNSAuthHandler_Permissions(t *testing.T) { "com.example.api.*", // subdomain pattern }, unexpectedPatterns: []string{ - "com.example/*", // parent domain should not be included - "api.example.com/*", // should be reversed + "com.example/*", // parent domain should not be included + "api." + testDomain + "/*", // should be reversed }, }, { @@ -367,7 +367,7 @@ func TestDNSAuthHandler_PermissionValidation(t *testing.T) { require.NoError(t, err) publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) - domain := "example.com" + domain := testDomain // Set up mock resolver mockResolver := &MockDNSResolver{ diff --git a/internal/api/handlers/v0/auth/http_test.go b/internal/api/handlers/v0/auth/http_test.go index f25619d64..7eeaef384 100644 --- a/internal/api/handlers/v0/auth/http_test.go +++ b/internal/api/handlers/v0/auth/http_test.go @@ -18,6 +18,8 @@ import ( "github.com/modelcontextprotocol/registry/internal/config" ) +const testDomain = "example.com" + // MockHTTPKeyFetcher for testing type MockHTTPKeyFetcher struct { keyResponses map[string]string @@ -45,7 +47,7 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) mockFetcher := &MockHTTPKeyFetcher{ keyResponses: map[string]string{ - "example.com": fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), + testDomain: fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", publicKeyB64), }, } handler.SetFetcher(mockFetcher) @@ -61,7 +63,7 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { }{ { name: "successful authentication", - domain: "example.com", + domain: testDomain, timestamp: time.Now().UTC().Format(time.RFC3339), setupMock: func(_ *MockHTTPKeyFetcher) { // Mock is already set up with valid key @@ -77,28 +79,28 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { }, { name: "invalid timestamp format", - domain: "example.com", + domain: testDomain, timestamp: "invalid-timestamp", expectError: true, errorContains: "invalid timestamp format", }, { name: "timestamp too old", - domain: "example.com", + domain: testDomain, timestamp: time.Now().Add(-30 * time.Second).UTC().Format(time.RFC3339), expectError: true, errorContains: "timestamp outside valid window", }, { name: "timestamp too far in the future", - domain: "example.com", + domain: testDomain, timestamp: time.Now().Add(30 * time.Second).UTC().Format(time.RFC3339), expectError: true, errorContains: "timestamp outside valid window", }, { name: "invalid signature format", - domain: "example.com", + domain: testDomain, timestamp: time.Now().UTC().Format(time.RFC3339), signedTimestamp: "invalid-hex", expectError: true, @@ -106,7 +108,7 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { }, { name: "signature wrong length", - domain: "example.com", + domain: testDomain, timestamp: time.Now().UTC().Format(time.RFC3339), signedTimestamp: "abcdef", // too short expectError: true, @@ -159,14 +161,14 @@ func TestHTTPAuthHandler_ExchangeToken(t *testing.T) { }, { name: "signature verification failure", - domain: "example.com", + domain: testDomain, timestamp: time.Now().UTC().Format(time.RFC3339), setupMock: func(m *MockHTTPKeyFetcher) { // Generate different key pair for signature verification failure wrongPublicKey, _, err := ed25519.GenerateKey(nil) require.NoError(t, err) wrongPublicKeyB64 := base64.StdEncoding.EncodeToString(wrongPublicKey) - m.keyResponses["example.com"] = fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", wrongPublicKeyB64) + m.keyResponses[testDomain] = fmt.Sprintf("v=MCPv1; k=ed25519; p=%s", wrongPublicKeyB64) m.err = nil }, expectError: true, @@ -263,7 +265,7 @@ func TestHTTPAuthHandler_Permissions(t *testing.T) { }{ { name: "simple domain", - domain: "example.com", + domain: testDomain, expectedPatterns: []string{ "com.example/*", // exact domain pattern only (HTTP does not include subdomains) }, @@ -411,7 +413,7 @@ func TestHTTPAuthHandler_PermissionValidation(t *testing.T) { require.NoError(t, err) publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) - domain := "example.com" + domain := testDomain // Set up mock fetcher mockFetcher := &MockHTTPKeyFetcher{ @@ -514,7 +516,7 @@ func TestHTTPvsDNS_PermissionDifferences(t *testing.T) { require.NoError(t, err) publicKeyB64 := base64.StdEncoding.EncodeToString(publicKey) - domain := "example.com" + domain := testDomain // Set up mocks mockFetcher := &MockHTTPKeyFetcher{ From d66b79b2122b73cf542682c54f6129693f5a1380 Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Mon, 29 Sep 2025 17:36:43 -0400 Subject: [PATCH 5/6] Address comments --- internal/api/handlers/v0/auth/common.go | 55 ++++++++++++++++++++++--- internal/api/handlers/v0/auth/dns.go | 43 ++++++------------- internal/api/handlers/v0/auth/http.go | 38 ++++------------- 3 files changed, 72 insertions(+), 64 deletions(-) diff --git a/internal/api/handlers/v0/auth/common.go b/internal/api/handlers/v0/auth/common.go index 3a05fb9cc..a94b7d33b 100644 --- a/internal/api/handlers/v0/auth/common.go +++ b/internal/api/handlers/v0/auth/common.go @@ -14,13 +14,16 @@ import ( "github.com/modelcontextprotocol/registry/internal/config" ) -// CoreTokenExchangeInput represents the common input structure for token exchange -type CoreTokenExchangeInput struct { +// SignatureTokenExchangeInput represents the common input structure for token exchange +type SignatureTokenExchangeInput struct { Domain string `json:"domain" doc:"Domain name" example:"example.com" required:"true"` Timestamp string `json:"timestamp" doc:"RFC3339 timestamp" example:"2023-01-01T00:00:00Z" required:"true"` SignedTimestamp string `json:"signed_timestamp" doc:"Hex-encoded Ed25519 signature of timestamp" example:"abcdef1234567890" required:"true"` } +// KeyFetcher defines a function type for fetching keys from external sources +type KeyFetcher func(ctx context.Context, domain string) ([]string, error) + // CoreAuthHandler represents the common handler structure type CoreAuthHandler struct { config *config.Config @@ -90,9 +93,6 @@ func BuildPermissions(domain string, includeSubdomains bool) []auth.Permission { } if includeSubdomains { - // DNS implies a hierarchy where subdomains are treated as part of the parent domain, - // therefore we grant permissions for all subdomains (e.g., com.example.*) - // This is in line with other DNS-based authentication methods e.g. ACME DNS-01 challenges permissions = append(permissions, auth.Permission{ Action: auth.PermissionActionPublish, ResourcePattern: fmt.Sprintf("%s.*", reverseDomain), @@ -120,6 +120,51 @@ func (h *CoreAuthHandler) CreateJWTClaimsAndToken(ctx context.Context, authMetho return tokenResponse, nil } +// ExchangeToken is a shared method for token exchange that takes a key fetcher function, +// subdomain inclusion flag, and auth method +func (h *CoreAuthHandler) ExchangeToken( + ctx context.Context, + domain, timestamp, signedTimestamp string, + keyFetcher KeyFetcher, + includeSubdomains bool, + authMethod auth.Method) (*auth.TokenResponse, error) { + _, err := ValidateDomainAndTimestamp(domain, timestamp) + if err != nil { + return nil, err + } + + signature, err := DecodeAndValidateSignature(signedTimestamp) + if err != nil { + return nil, err + } + + keyStrings, err := keyFetcher(ctx, domain) + if err != nil { + return nil, fmt.Errorf("failed to fetch keys: %w", err) + } + + publicKeys := ParseMCPKeysFromStrings(keyStrings) + if len(publicKeys) == 0 { + switch authMethod { + case auth.MethodHTTP: + return nil, fmt.Errorf("failed to parse public key") + case auth.MethodDNS: + return nil, fmt.Errorf("no valid MCP public keys found in DNS TXT records") + default: + return nil, fmt.Errorf("no valid MCP public keys found using %s authentication", authMethod) + } + } + + messageBytes := []byte(timestamp) + if !VerifySignatureWithKeys(publicKeys, messageBytes, signature) { + return nil, fmt.Errorf("signature verification failed") + } + + permissions := BuildPermissions(domain, includeSubdomains) + + return h.CreateJWTClaimsAndToken(ctx, authMethod, domain, permissions) +} + func ParseMCPKeysFromStrings(inputs []string) []ed25519.PublicKey { var publicKeys []ed25519.PublicKey mcpPattern := regexp.MustCompile(`v=MCPv1;\s*k=ed25519;\s*p=([A-Za-z0-9+/=]+)`) diff --git a/internal/api/handlers/v0/auth/dns.go b/internal/api/handlers/v0/auth/dns.go index cce36fb40..99c7a3cc2 100644 --- a/internal/api/handlers/v0/auth/dns.go +++ b/internal/api/handlers/v0/auth/dns.go @@ -14,7 +14,7 @@ import ( // DNSTokenExchangeInput represents the input for DNS-based authentication type DNSTokenExchangeInput struct { - Body CoreTokenExchangeInput + Body SignatureTokenExchangeInput } // DNSResolver defines the interface for DNS resolution @@ -75,35 +75,18 @@ func RegisterDNSEndpoint(api huma.API, cfg *config.Config) { // ExchangeToken exchanges DNS signature for a Registry JWT token func (h *DNSAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { - _, err := ValidateDomainAndTimestamp(domain, timestamp) - if err != nil { - return nil, err - } - - signature, err := DecodeAndValidateSignature(signedTimestamp) - if err != nil { - return nil, err - } - - // Lookup DNS TXT records - txtRecords, err := h.resolver.LookupTXT(ctx, domain) - if err != nil { - return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err) - } - - publicKeys := ParseMCPKeysFromStrings(txtRecords) - - if len(publicKeys) == 0 { - return nil, fmt.Errorf("no valid MCP public keys found in DNS TXT records") - } - - messageBytes := []byte(timestamp) - if !VerifySignatureWithKeys(publicKeys, messageBytes, signature) { - return nil, fmt.Errorf("signature verification failed") + keyFetcher := func(ctx context.Context, domain string) ([]string, error) { + // Lookup DNS TXT records + // DNS implies a hierarchy where subdomains are treated as part of the parent domain, + // therefore we grant permissions for all subdomains (e.g., com.example.*) + // This is in line with other DNS-based authentication methods e.g. ACME DNS-01 challenges + txtRecords, err := h.resolver.LookupTXT(ctx, domain) + if err != nil { + return nil, fmt.Errorf("failed to lookup DNS TXT records: %w", err) + } + return txtRecords, nil } - // Build permissions for domain (DNS includes subdomains) - permissions := BuildPermissions(domain, true) - - return h.CreateJWTClaimsAndToken(ctx, auth.MethodDNS, domain, permissions) + allowSubdomains := true + return h.CoreAuthHandler.ExchangeToken(ctx, domain, timestamp, signedTimestamp, keyFetcher, allowSubdomains, auth.MethodDNS) } diff --git a/internal/api/handlers/v0/auth/http.go b/internal/api/handlers/v0/auth/http.go index 6d6a108f5..d7acebfeb 100644 --- a/internal/api/handlers/v0/auth/http.go +++ b/internal/api/handlers/v0/auth/http.go @@ -19,7 +19,7 @@ const MaxKeyResponseSize = 4096 // HTTPTokenExchangeInput represents the input for HTTP-based authentication type HTTPTokenExchangeInput struct { - Body CoreTokenExchangeInput + Body SignatureTokenExchangeInput } // HTTPKeyFetcher defines the interface for fetching HTTP keys @@ -133,34 +133,14 @@ func RegisterHTTPEndpoint(api huma.API, cfg *config.Config) { // ExchangeToken exchanges HTTP signature for a Registry JWT token func (h *HTTPAuthHandler) ExchangeToken(ctx context.Context, domain, timestamp, signedTimestamp string) (*auth.TokenResponse, error) { - _, err := ValidateDomainAndTimestamp(domain, timestamp) - if err != nil { - return nil, err - } - - signature, err := DecodeAndValidateSignature(signedTimestamp) - if err != nil { - return nil, err - } - - // Fetch the key from well known HTTP endpoint - keyResponse, err := h.fetcher.FetchKey(ctx, domain) - if err != nil { - return nil, fmt.Errorf("failed to fetch public key: %w", err) - } - - publicKeys := ParseMCPKeysFromStrings([]string{keyResponse}) - if len(publicKeys) == 0 { - return nil, fmt.Errorf("failed to parse public key") - } - - messageBytes := []byte(timestamp) - if !VerifySignatureWithKeys(publicKeys, messageBytes, signature) { - return nil, fmt.Errorf("signature verification failed") + keyFetcher := func(ctx context.Context, domain string) ([]string, error) { + keyResponse, err := h.fetcher.FetchKey(ctx, domain) + if err != nil { + return nil, fmt.Errorf("failed to fetch public key: %w", err) + } + return []string{keyResponse}, nil } - // Build permissions for domain (HTTP does not include subdomains) - permissions := BuildPermissions(domain, false) - - return h.CreateJWTClaimsAndToken(ctx, auth.MethodHTTP, domain, permissions) + allowSubdomains := false + return h.CoreAuthHandler.ExchangeToken(ctx, domain, timestamp, signedTimestamp, keyFetcher, allowSubdomains, auth.MethodHTTP) } From 3e83cc88fb7262ba985e1ffabf9364154f46f1a5 Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Mon, 29 Sep 2025 18:05:54 -0400 Subject: [PATCH 6/6] Fix lint issue --- internal/api/handlers/v0/auth/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/handlers/v0/auth/common.go b/internal/api/handlers/v0/auth/common.go index a94b7d33b..e290a234c 100644 --- a/internal/api/handlers/v0/auth/common.go +++ b/internal/api/handlers/v0/auth/common.go @@ -150,7 +150,7 @@ func (h *CoreAuthHandler) ExchangeToken( return nil, fmt.Errorf("failed to parse public key") case auth.MethodDNS: return nil, fmt.Errorf("no valid MCP public keys found in DNS TXT records") - default: + case auth.MethodGitHubAT, auth.MethodGitHubOIDC, auth.MethodOIDC, auth.MethodNone: return nil, fmt.Errorf("no valid MCP public keys found using %s authentication", authMethod) } }