diff --git a/plugin/pkg/auth/authenticator/token/oidc/oidc.go b/plugin/pkg/auth/authenticator/token/oidc/oidc.go index 4a0e191ca9b14..4a6728f86a229 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/oidc.go +++ b/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) diff --git a/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go b/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go index 4956e5da33892..9195d2420f1a8 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/oidc_test.go +++ b/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,119 +185,120 @@ func TestOIDCAuthentication(t *testing.T) { oidctesting.GenerateSelfSignedCert(t, "127.0.0.1", cert, key) - // Create a TLS server and a client. - op := oidctesting.NewOIDCProvider(t) - srv, err := op.ServeTLSWithKeyPair(cert, key) - if err != nil { - t.Fatalf("Cannot start server: %v", err) - } - defer srv.Close() + // Ensure all tests pass when the issuer is not at a base URL. + for _, path := range []string{"", "/path/with/trailing/slash/"} { - // A provider config with all required fields. - op.AddMinimalProviderConfig(srv) - - tests := []struct { - userClaim string - groupsClaim string - token string - userInfo user.Info - verified bool - err string - }{ - { - "sub", - "", - generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), - &user.DefaultInfo{Name: fmt.Sprintf("%s#%s", srv.URL, "user-foo")}, - true, - "", - }, - { - // Use user defined claim (email here). - "email", - "", - generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "email", "foo@example.com", "", nil), - &user.DefaultInfo{Name: "foo@example.com"}, - true, - "", - }, - { - // Use user defined claim (email here). - "email", - "", - generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "email", "foo@example.com", "groups", []string{"group1", "group2"}), - &user.DefaultInfo{Name: "foo@example.com"}, - true, - "", - }, - { - // Use user defined claim (email here). - "email", - "groups", - generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "email", "foo@example.com", "groups", []string{"group1", "group2"}), - &user.DefaultInfo{Name: "foo@example.com", Groups: []string{"group1", "group2"}}, - true, - "", - }, - { - "sub", - "", - generateMalformedToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), - nil, - false, - "oidc: unable to verify JWT signature: no matching keys", - }, - { - // Invalid 'aud'. - "sub", - "", - generateGoodToken(t, op, srv.URL, "client-foo", "client-bar", "sub", "user-foo", "", nil), - nil, - false, - "oidc: JWT claims invalid: invalid claims, 'aud' claim and 'client_id' do not match", - }, - { - // Invalid issuer. - "sub", - "", - generateGoodToken(t, op, "http://foo-bar.com", "client-foo", "client-foo", "sub", "user-foo", "", nil), - nil, - false, - "oidc: JWT claims invalid: invalid claim value: 'iss'.", - }, - { - "sub", - "", - generateExpiredToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), - nil, - false, - "oidc: JWT claims invalid: token is expired", - }, - } - - for i, tt := range tests { - client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim}) + // Create a TLS server and a client. + op := oidctesting.NewOIDCProvider(t, path) + srv, err := op.ServeTLSWithKeyPair(cert, key) if err != nil { - t.Errorf("Unexpected error: %v", err) - continue + t.Fatalf("Cannot start server: %v", err) + } + defer srv.Close() + + tests := []struct { + userClaim string + groupsClaim string + token string + userInfo user.Info + verified bool + err string + }{ + { + "sub", + "", + generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), + &user.DefaultInfo{Name: fmt.Sprintf("%s#%s", srv.URL, "user-foo")}, + true, + "", + }, + { + // Use user defined claim (email here). + "email", + "", + generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "email", "foo@example.com", "", nil), + &user.DefaultInfo{Name: "foo@example.com"}, + true, + "", + }, + { + // Use user defined claim (email here). + "email", + "", + generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "email", "foo@example.com", "groups", []string{"group1", "group2"}), + &user.DefaultInfo{Name: "foo@example.com"}, + true, + "", + }, + { + // Use user defined claim (email here). + "email", + "groups", + generateGoodToken(t, op, srv.URL, "client-foo", "client-foo", "email", "foo@example.com", "groups", []string{"group1", "group2"}), + &user.DefaultInfo{Name: "foo@example.com", Groups: []string{"group1", "group2"}}, + true, + "", + }, + { + "sub", + "", + generateMalformedToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), + nil, + false, + "oidc: unable to verify JWT signature: no matching keys", + }, + { + // Invalid 'aud'. + "sub", + "", + generateGoodToken(t, op, srv.URL, "client-foo", "client-bar", "sub", "user-foo", "", nil), + nil, + false, + "oidc: JWT claims invalid: invalid claims, 'aud' claim and 'client_id' do not match", + }, + { + // Invalid issuer. + "sub", + "", + generateGoodToken(t, op, "http://foo-bar.com", "client-foo", "client-foo", "sub", "user-foo", "", nil), + nil, + false, + "oidc: JWT claims invalid: invalid claim value: 'iss'.", + }, + { + "sub", + "", + generateExpiredToken(t, op, srv.URL, "client-foo", "client-foo", "sub", "user-foo", "", nil), + nil, + false, + "oidc: JWT claims invalid: token is expired", + }, } - user, result, err := client.AuthenticateToken(tt.token) - if tt.err != "" { - if !strings.HasPrefix(err.Error(), tt.err) { - t.Errorf("#%d: Expecting: %v..., but got: %v", i, tt.err, err) - } - } else { + for i, tt := range tests { + client, err := New(OIDCOptions{srv.URL, "client-foo", cert, tt.userClaim, tt.groupsClaim}) if err != nil { - t.Errorf("#%d: Unexpected error: %v", i, err) + t.Errorf("Unexpected error: %v", err) + continue } + + user, result, err := client.AuthenticateToken(tt.token) + if tt.err != "" { + if !strings.HasPrefix(err.Error(), tt.err) { + t.Errorf("#%d: Expecting: %v..., but got: %v", i, tt.err, err) + } + } else { + if err != nil { + t.Errorf("#%d: Unexpected error: %v", i, err) + } + } + if !reflect.DeepEqual(tt.verified, result) { + t.Errorf("#%d: Expecting: %v, but got: %v", i, tt.verified, result) + } + if !reflect.DeepEqual(tt.userInfo, user) { + t.Errorf("#%d: Expecting: %v, but got: %v", i, tt.userInfo, user) + } + client.Close() } - if !reflect.DeepEqual(tt.verified, result) { - t.Errorf("#%d: Expecting: %v, but got: %v", i, tt.verified, result) - } - if !reflect.DeepEqual(tt.userInfo, user) { - t.Errorf("#%d: Expecting: %v, but got: %v", i, tt.userInfo, user) - } - client.Close() } } diff --git a/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go b/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go index b3cbdc7a3a584..ae7353ff276e7 100644 --- a/plugin/pkg/auth/authenticator/token/oidc/testing/provider.go +++ b/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) @@ -51,20 +52,22 @@ func NewOIDCProvider(t *testing.T) *OIDCProvider { } op := &OIDCProvider{ - Mux: http.NewServeMux(), - PrivKey: privKey, + 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 } type OIDCProvider struct { - Mux *http.ServeMux - PCFG oidc.ProviderConfig - PrivKey *key.PrivateKey + 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). diff --git a/plugin/pkg/client/auth/oidc/oidc_test.go b/plugin/pkg/client/auth/oidc/oidc_test.go index 4e15113b95795..e3dca0f748e44 100644 --- a/plugin/pkg/client/auth/oidc/oidc_test.go +++ b/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 {