Add endpointaddr.ParseFromURL helper, WebhookAuthenticator handle additional IPv6 cases

2026-01-07 14:05:50 +00:00 · 2024-03-22 11:57:05 -04:00
parent b0904c2e99
commit e38a27d93d
4 changed files with 468 additions and 69 deletions
--- a/internal/endpointaddr/endpointaddr.go
+++ b/internal/endpointaddr/endpointaddr.go
@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 // Package endpointaddr implements parsing and validation of "<host>[:<port>]" strings for Pinniped APIs.
@@ -7,7 +7,9 @@ package endpointaddr
 import (
 	"fmt"
 	"net"
+	"net/url"
 	"strconv"
+	"strings"

 	"k8s.io/apimachinery/pkg/util/validation"
 )
@@ -38,7 +40,7 @@ func (h *HostPort) Endpoint() string {
 // - "<IPv4>:<port>"     (IPv4 address with port)
 // - "[<IPv6>]:<port>"   (IPv6 address with port, brackets are required)
 //
-// If the input does not not specify a port number, then defaultPort will be used.
+// If the input does not specify a port number, then defaultPort will be used.
 func Parse(endpoint string, defaultPort uint16) (HostPort, error) {
 	// Try parsing it both with and without an implicit port 443 at the end.
 	host, port, err := net.SplitHostPort(endpoint)
@@ -69,3 +71,36 @@ func Parse(endpoint string, defaultPort uint16) (HostPort, error) {

 	return HostPort{Host: host, Port: uint16(integerPort)}, nil
 }
+
+// ParseFromURL wraps Parse but specifically takes a url.URL instead of an endpoint string.
+// ParseFromURL differs from Parse in that a URL will contain a protocol, and IPv6 addresses
+// may or may not be wrapped in brackets (but require them when a port is provided):
+//
+// - "https://<hostname>"        (DNS hostname)
+// - "https://<IPv4>"            (IPv4 address)
+// - "https://<IPv6>"            (IPv6 address)
+// - "https://[<IPv6>]"          (IPv6 address without port, brackets should be used but are not strictly required)
+// - "https://<hostname>:<port>" (DNS hostname with port)
+// - "https://<IPv4>:<port>"     (IPv4 address with port)
+// - "https://[<IPv6>]:<port>"   (IPv6 address with port, brackets are required)
+//
+// If the input does not specify a port number, then defaultPort will be used.
+//
+// The rfc for literal IPv6 addresses in URLs indicates that brackets
+// - must be used when a port is provided
+// - should be used when a port is not provided, but does not indicate "must"
+//
+// Since url.Parse does not inspect the host, it will accept IPv6 hosts without
+// brackets and without port, which may result in errors that are not immediately obvious.
+// Therefore, this helper will normalize the bracketed use case.  Note that this is
+// because ParseFromURL returns a HostPort which has an Endpoint() method which will
+// return a properly constructed URL with brackets when appropriate.
+//
+// See RFC: https://datatracker.ietf.org/doc/html/rfc2732#section-2
+func ParseFromURL(u *url.URL, defaultPort uint16) (HostPort, error) {
+	host := u.Host
+	if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") {
+		host = strings.TrimPrefix(strings.TrimSuffix(host, "]"), "[")
+	}
+	return Parse(host, defaultPort)
+}
--- a/internal/endpointaddr/endpointaddr_test.go
+++ b/internal/endpointaddr/endpointaddr_test.go
@@ -1,12 +1,14 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package endpointaddr

 import (
+	"net/url"
 	"testing"

 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )

 func TestParse(t *testing.T) {
@@ -180,3 +182,170 @@ func TestParse(t *testing.T) {
 		})
 	}
 }
+
+func TestParseFromURL(t *testing.T) {
+	t.Parallel()
+	for _, tt := range []struct {
+		name        string
+		input       string
+		defaultPort uint16
+		expectErr   string
+		expect      HostPort
+		// HostPort.Endpoint() returns a properly constructed endpoint.  The normalization provided by ParseFromURL()
+		// expects that the resulting HostPort.Endpoint() will be called to normalize several special cases, especially
+		// for IPv6.
+		expectEndpoint string
+	}{
+		// First set of valid passthrough tests to Parse()
+		// Matches the above test table, minus any test that would not url.Parse(input) properly
+		{
+			name:           "plain IPv4",
+			input:          "http://127.0.0.1",
+			defaultPort:    443,
+			expect:         HostPort{Host: "127.0.0.1", Port: 443},
+			expectEndpoint: "127.0.0.1:443",
+		},
+		{
+			name:           "IPv4 with port",
+			input:          "http://127.0.0.1:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "127.0.0.1", Port: 8443},
+			expectEndpoint: "127.0.0.1:8443",
+		},
+		{
+			name:           "IPv4 in brackets with port",
+			input:          "http://[127.0.0.1]:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "127.0.0.1", Port: 8443},
+			expectEndpoint: "127.0.0.1:8443",
+		},
+		{
+			name:           "IPv4 as IPv6 in brackets with port",
+			input:          "http://[::127.0.0.1]:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "::127.0.0.1", Port: 8443},
+			expectEndpoint: "[::127.0.0.1]:8443",
+		},
+		{
+			name:           "IPv6 with port",
+			input:          "http://[2001:db8::ffff]:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "2001:db8::ffff", Port: 8443},
+			expectEndpoint: "[2001:db8::ffff]:8443",
+		},
+		{
+			name:           "plain hostname",
+			input:          "http://host.example.com",
+			defaultPort:    443,
+			expect:         HostPort{Host: "host.example.com", Port: 443},
+			expectEndpoint: "host.example.com:443",
+		},
+		{
+			name:           "plain hostname with dash",
+			input:          "http://host-dev.example.com",
+			defaultPort:    443,
+			expect:         HostPort{Host: "host-dev.example.com", Port: 443},
+			expectEndpoint: "host-dev.example.com:443",
+		},
+		{
+			name:           "hostname with port",
+			input:          "http://host.example.com:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "host.example.com", Port: 8443},
+			expectEndpoint: "host.example.com:8443",
+		},
+		{
+			name:           "hostname in brackets with port",
+			input:          "http://[host.example.com]:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "host.example.com", Port: 8443},
+			expectEndpoint: "host.example.com:8443",
+		},
+		{
+			name:           "hostname without dots",
+			input:          "http://localhost",
+			defaultPort:    443,
+			expect:         HostPort{Host: "localhost", Port: 443},
+			expectEndpoint: "localhost:443",
+		},
+		{
+			name:           "hostname and port without dots",
+			input:          "http://localhost:8443",
+			defaultPort:    443,
+			expect:         HostPort{Host: "localhost", Port: 8443},
+			expectEndpoint: "localhost:8443",
+		},
+		{
+			name:        "http://invalid empty string",
+			input:       "",
+			defaultPort: 443,
+			expectErr:   `host "" is not a valid hostname or IP address`,
+		},
+		{
+			name:        "invalid host with underscores",
+			input:       "http://___.example.com:1234",
+			defaultPort: 443,
+			expectErr:   `host "___.example.com" is not a valid hostname or IP address`,
+		},
+		{
+			name:        "invalid host with uppercase",
+			input:       "http://HOST.EXAMPLE.COM",
+			defaultPort: 443,
+			expectErr:   `host "HOST.EXAMPLE.COM" is not a valid hostname or IP address`,
+		},
+		// new tests for new functionality
+		{
+			name:           "IPv6 with brackets but without port will strip brackets to create HostPort{}, which will add brackets when HostPort.Endpoint() is called",
+			input:          "http://[2001:db8::ffff]",
+			defaultPort:    443,
+			expect:         HostPort{Host: "2001:db8::ffff", Port: 443},
+			expectEndpoint: "[2001:db8::ffff]:443",
+		},
+		{
+			name:           "IPv6 without brackets and without port will create HostPort{}, which will add brackets when HostPort.Endpoint() is called",
+			input:          "http://2001:db8::1234",
+			defaultPort:    443,
+			expect:         HostPort{Host: "2001:db8::1234", Port: 443},
+			expectEndpoint: "[2001:db8::1234]:443",
+		},
+		{
+			name:           "IPv6 without brackets and without port with path create HostPort{}, which will add brackets when HostPort.Endpoint() is called",
+			input:          "https://0:0:0:0:0:0:0:1/some/fake/path",
+			defaultPort:    443,
+			expect:         HostPort{Host: "0:0:0:0:0:0:0:1", Port: 443},
+			expectEndpoint: "[0:0:0:0:0:0:0:1]:443",
+		},
+		{
+			name:           "IPv6 with mismatched leading bracket will err on bracket",
+			input:          "https://[[::1]/some/fake/path",
+			defaultPort:    443,
+			expect:         HostPort{Host: "[[::1]", Port: 443},
+			expectEndpoint: "[[::1]:443",
+			expectErr:      `address [[::1]:443: unexpected '[' in address`,
+		},
+		{
+			name:           "IPv6 with mismatched trailing brackets will err on port",
+			input:          "https://[::1]]/some/fake/path",
+			defaultPort:    443,
+			expect:         HostPort{Host: "[::1]]", Port: 443},
+			expectEndpoint: "[::1]]:443",
+			expectErr:      `address [::1]]:443: missing port in address`,
+		},
+	} {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			urlToProcess, err := url.Parse(tt.input)
+			require.NoError(t, err, "ParseFromURL expects a valid url.URL, parse errors here are not valuable")
+
+			got, err := ParseFromURL(urlToProcess, tt.defaultPort)
+			if tt.expectErr == "" {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expect, got)
+				assert.Equal(t, tt.expectEndpoint, got.Endpoint())
+			} else {
+				assert.EqualError(t, err, tt.expectErr)
+				assert.Equal(t, HostPort{}, got)
+			}
+		})
+	}
+}