oidc authentication plugin: don't trim issuer URLs with trailing slashes

The issuer URL passed to the plugin must identically match the issuer
URL returned by OpenID Connect discovery. However, the plugin currently
trims all trailing slashes from issuer URLs, causing a mismatch. Since
the go-oidc client already handles this case correctly, don't trim the
path.

plugin/pkg/auth/authenticator/token/oidc/oidc.go

@@ -33,7 +33,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"strings"
 	"sync"
 	"sync/atomic"
 
@@ -174,7 +173,7 @@ func (a *OIDCAuthenticator) client() (*oidc.Client, error) {
 	}
 
 	// Try to initialize client.
-	providerConfig, err := oidc.FetchProviderConfig(a.httpClient, strings.TrimSuffix(a.issuerURL, "/"))
+	providerConfig, err := oidc.FetchProviderConfig(a.httpClient, a.issuerURL)
 	if err != nil {
 		glog.Errorf("oidc authenticator: failed to fetch provider discovery data: %v", err)
 		return nil, fmt.Errorf("fetch provider config: %v", err)

plugin/pkg/auth/authenticator/token/oidc/oidc_test.go

@@ -110,14 +110,13 @@ func TestTLSConfig(t *testing.T) {
 
 	for _, tc := range tests {
 		func() {
-			op := oidctesting.NewOIDCProvider(t)
+			op := oidctesting.NewOIDCProvider(t, "")
 			srv, err := op.ServeTLSWithKeyPair(tc.serverCertFile, tc.serverKeyFile)
 			if err != nil {
 				t.Errorf("%s: %v", tc.testCase, err)
 				return
 			}
 			defer srv.Close()
-			op.AddMinimalProviderConfig(srv)
 
 			issuer := srv.URL
 			clientID := "client-foo"
@@ -178,8 +177,6 @@ func TestTLSConfig(t *testing.T) {
 }
 
 func TestOIDCAuthentication(t *testing.T) {
-	var err error
-
 	cert := path.Join(os.TempDir(), "oidc-cert")
 	key := path.Join(os.TempDir(), "oidc-key")
 
@@ -188,17 +185,17 @@ func TestOIDCAuthentication(t *testing.T) {
 
 	oidctesting.GenerateSelfSignedCert(t, "127.0.0.1", cert, key)
 
+	// Ensure all tests pass when the issuer is not at a base URL.
+	for _, path := range []string{"", "/path/with/trailing/slash/"} {
+
 		// Create a TLS server and a client.
-	op := oidctesting.NewOIDCProvider(t)
+		op := oidctesting.NewOIDCProvider(t, path)
 		srv, err := op.ServeTLSWithKeyPair(cert, key)
 		if err != nil {
 			t.Fatalf("Cannot start server: %v", err)
 		}
 		defer srv.Close()
 
-	// A provider config with all required fields.
-	op.AddMinimalProviderConfig(srv)
-
 		tests := []struct {
 			userClaim   string
 			groupsClaim string
@@ -303,4 +300,5 @@ func TestOIDCAuthentication(t *testing.T) {
 			}
 			client.Close()
 		}
+	}
 }

plugin/pkg/auth/authenticator/token/oidc/testing/provider.go

@@ -33,6 +33,7 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"os"
+	"path"
 	"path/filepath"
 	"testing"
 	"time"
@@ -43,7 +44,7 @@ import (
 )
 
 // NewOIDCProvider provides a bare minimum OIDC IdP Server useful for testing.
-func NewOIDCProvider(t *testing.T) *OIDCProvider {
+func NewOIDCProvider(t *testing.T, issuerPath string) *OIDCProvider {
 	privKey, err := key.GeneratePrivateKey()
 	if err != nil {
 		t.Fatalf("Cannot create OIDC Provider: %v", err)
@@ -53,10 +54,11 @@ func NewOIDCProvider(t *testing.T) *OIDCProvider {
 	op := &OIDCProvider{
 		Mux:        http.NewServeMux(),
 		PrivKey:    privKey,
+		issuerPath: issuerPath,
 	}
 
-	op.Mux.HandleFunc("/.well-known/openid-configuration", op.handleConfig)
-	op.Mux.HandleFunc("/keys", op.handleKeys)
+	op.Mux.HandleFunc(path.Join(issuerPath, "/.well-known/openid-configuration"), op.handleConfig)
+	op.Mux.HandleFunc(path.Join(issuerPath, "/keys"), op.handleKeys)
 
 	return op
 }
@@ -65,6 +67,7 @@ type OIDCProvider struct {
 	Mux        *http.ServeMux
 	PCFG       oidc.ProviderConfig
 	PrivKey    *key.PrivateKey
+	issuerPath string
 }
 
 func (op *OIDCProvider) ServeTLSWithKeyPair(cert, key string) (*httptest.Server, error) {
@@ -77,20 +80,31 @@ func (op *OIDCProvider) ServeTLSWithKeyPair(cert, key string) (*httptest.Server,
 		return nil, fmt.Errorf("Cannot load cert/key pair: %v", err)
 	}
 	srv.StartTLS()
-	return srv, nil
-}
 
-func (op *OIDCProvider) AddMinimalProviderConfig(srv *httptest.Server) {
+	// The issuer's URL is extended by an optional path. This ensures that the plugin can
+	// handle issuers that use a non-root path for discovery (see kubernetes/kubernetes#29749).
+	srv.URL = srv.URL + op.issuerPath
+
+	u, err := url.Parse(srv.URL)
+	if err != nil {
+		return nil, err
+	}
+	pathFor := func(p string) *url.URL {
+		u2 := *u // Shallow copy.
+		u2.Path = path.Join(u2.Path, p)
+		return &u2
+	}
+
 	op.PCFG = oidc.ProviderConfig{
-		Issuer:                  MustParseURL(srv.URL),
-		AuthEndpoint:            MustParseURL(srv.URL + "/auth"),
-		TokenEndpoint:           MustParseURL(srv.URL + "/token"),
-		KeysEndpoint:            MustParseURL(srv.URL + "/keys"),
+		Issuer:                  u,
+		AuthEndpoint:            pathFor("/auth"),
+		TokenEndpoint:           pathFor("/token"),
+		KeysEndpoint:            pathFor("/keys"),
 		ResponseTypesSupported:  []string{"code"},
 		SubjectTypesSupported:   []string{"public"},
 		IDTokenSigningAlgValues: []string{"RS256"},
 	}
-
+	return srv, nil
 }
 
 func (op *OIDCProvider) handleConfig(w http.ResponseWriter, req *http.Request) {
@@ -122,14 +136,6 @@ func (op *OIDCProvider) handleKeys(w http.ResponseWriter, req *http.Request) {
 	w.Write(b)
 }
 
-func MustParseURL(s string) *url.URL {
-	u, err := url.Parse(s)
-	if err != nil {
-		panic(fmt.Errorf("Failed to parse url: %v", err))
-	}
-	return u
-}
-
 // generateSelfSignedCert generates a self-signed cert/key pairs and writes to the certPath/keyPath.
 // This method is mostly identical to crypto.GenerateSelfSignedCert except for the 'IsCA' and 'KeyUsage'
 // in the certificate template. (Maybe we can merge these two methods).

plugin/pkg/client/auth/oidc/oidc_test.go

@@ -51,13 +51,12 @@ func TestNewOIDCAuthProvider(t *testing.T) {
 	defer os.Remove(tempDir)
 
 	oidctesting.GenerateSelfSignedCert(t, "127.0.0.1", cert, key)
-	op := oidctesting.NewOIDCProvider(t)
+	op := oidctesting.NewOIDCProvider(t, "")
 	srv, err := op.ServeTLSWithKeyPair(cert, key)
 	if err != nil {
 		t.Fatalf("Cannot start server %v", err)
 	}
 	defer srv.Close()
-	op.AddMinimalProviderConfig(srv)
 
 	certData, err := ioutil.ReadFile(cert)
 	if err != nil {