Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 4, 2016
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions plugin/pkg/auth/authenticator/token/oidc/oidc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
"fmt"
"net/http"
"net/url"
"strings"
"sync"
"sync/atomic"

Expand Down Expand Up @@ -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)
Expand Down
220 changes: 109 additions & 111 deletions plugin/pkg/auth/authenticator/token/oidc/oidc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")

Expand All @@ -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()
}
}
54 changes: 30 additions & 24 deletions plugin/pkg/auth/authenticator/token/oidc/testing/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"net/http/httptest"
"net/url"
"os"
"path"
"path/filepath"
"testing"
"time"
Expand All @@ -43,28 +44,30 @@ 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)
return nil
}

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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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).
Expand Down
3 changes: 1 addition & 2 deletions plugin/pkg/client/auth/oidc/oidc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down