djpbessems
/
ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #385 from vmware-tanzu/credential_request_spec_api_group 

Use custom suffix in `Spec.Authenticator.APIGroup` of `TokenCredentialRequest`
This commit is contained in:
Matt Moyer 2021-02-04 16:19:20 -06:00 committed by GitHub
parent bb8b65cca6 2a921f7090
commit 9addb4d6e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 203 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/concierge/server/server.go
View File

@ -110,7 +110,7 @@ func (a *App) runServer(ctx context.Context) error {
}
// Initialize the cache of active authenticators.
authenticators := authncache.New()
authenticators := authncache.New(*cfg.APIGroupSuffix)
// This cert provider will provide certs to the API server and will
// be mutated by a controller to keep the certs up to date with what

15
internal/controller/authenticator/authncache/cache.go
View File

@ -15,6 +15,7 @@ import (
"k8s.io/klog/v2"
loginapi "go.pinniped.dev/generated/1.20/apis/concierge/login"
"go.pinniped.dev/internal/groupsuffix"
"go.pinniped.dev/internal/plog"
)
@ -26,7 +27,8 @@ var (
// Cache implements the authenticator.Token interface by multiplexing across a dynamic set of authenticators
// loaded from authenticator resources.
type Cache struct {
cache sync.Map
cache sync.Map
apiGroupSuffix string
}
type Key struct {
@ -41,8 +43,8 @@ type Value interface {
}
// New returns an empty cache.
func New() *Cache {
return &Cache{}
func New(apiGroupSuffix string) *Cache {
return &Cache{apiGroupSuffix: apiGroupSuffix}
}
// Get an authenticator by key.
@ -90,7 +92,12 @@ func (c *Cache) AuthenticateTokenCredentialRequest(ctx context.Context, req *log
Kind: req.Spec.Authenticator.Kind,
}
if req.Spec.Authenticator.APIGroup != nil {
key.APIGroup = *req.Spec.Authenticator.APIGroup
// The key must always be API group pinniped.dev because that's what the cache filler will always use.
apiGroup, replaced := groupsuffix.Unreplace(*req.Spec.Authenticator.APIGroup, c.apiGroupSuffix)
if !replaced {
return nil, ErrNoSuchAuthenticator
}
key.APIGroup = apiGroup
}
val := c.Get(key)

70
internal/controller/authenticator/authncache/cache_test.go
View File

@ -28,7 +28,7 @@ func TestCache(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
cache := New()
cache := New("pinniped.dev")
require.NotNil(t, cache)
key1 := Key{Namespace: "foo", Name: "authenticator-one"}
@ -57,7 +57,7 @@ func TestCache(t *testing.T) {
{APIGroup: "b", Kind: "b", Namespace: "b", Name: "b"},
}
for tries := 0; tries < 10; tries++ {
cache := New()
cache := New("pinniped.dev")
for _, i := range rand.Perm(len(keysInExpectedOrder)) {
cache.Store(keysInExpectedOrder[i], nil)
}
@ -91,46 +91,48 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) {
Name: validRequest.Spec.Authenticator.Name,
}
mockCache := func(t *testing.T, res *authenticator.Response, authenticated bool, err error) *Cache {
mockCache := func(t *testing.T, apiGroupSuffix string, expectAuthenticatorToBeCalled bool, res *authenticator.Response, authenticated bool, err error) *Cache {
ctrl := gomock.NewController(t)
t.Cleanup(ctrl.Finish)
m := mocktokenauthenticator.NewMockToken(ctrl)
m.EXPECT().AuthenticateToken(audienceFreeContext{}, validRequest.Spec.Token).Return(res, authenticated, err)
c := New()
if expectAuthenticatorToBeCalled {
m.EXPECT().AuthenticateToken(audienceFreeContext{}, validRequest.Spec.Token).Return(res, authenticated, err)
}
c := New(apiGroupSuffix)
c.Store(validRequestKey, m)
return c
}
t.Run("no such authenticator", func(t *testing.T) {
c := New()
c := New("pinniped.dev")
res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy())
require.EqualError(t, err, "no such authenticator")
require.Nil(t, res)
})
t.Run("authenticator returns error", func(t *testing.T) {
c := mockCache(t, nil, false, fmt.Errorf("some authenticator error"))
c := mockCache(t, "pinniped.dev", true, nil, false, fmt.Errorf("some authenticator error"))
res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy())
require.EqualError(t, err, "some authenticator error")
require.Nil(t, res)
})
t.Run("authenticator returns unauthenticated without error", func(t *testing.T) {
c := mockCache(t, &authenticator.Response{}, false, nil)
c := mockCache(t, "pinniped.dev", true, &authenticator.Response{}, false, nil)
res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy())
require.NoError(t, err)
require.Nil(t, res)
})
t.Run("authenticator returns nil response without error", func(t *testing.T) {
c := mockCache(t, nil, true, nil)
c := mockCache(t, "pinniped.dev", true, nil, true, nil)
res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy())
require.NoError(t, err)
require.Nil(t, res)
})
t.Run("authenticator returns response with nil user", func(t *testing.T) {
c := mockCache(t, &authenticator.Response{}, true, nil)
c := mockCache(t, "pinniped.dev", true, &authenticator.Response{}, true, nil)
res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy())
require.NoError(t, err)
require.Nil(t, res)
@ -151,7 +153,7 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) {
}
},
)
c := New()
c := New("pinniped.dev")
c.Store(validRequestKey, m)
ctx, cancel := context.WithCancel(context.Background())
@ -171,7 +173,7 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) {
Groups: []string{"test-group-1", "test-group-2"},
Extra: map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}},
}
c := mockCache(t, &authenticator.Response{User: &userInfo}, true, nil)
c := mockCache(t, "pinniped.dev", true, &authenticator.Response{User: &userInfo}, true, nil)
audienceCtx := authenticator.WithAudiences(context.Background(), authenticator.Audiences{"test-audience-1"})
res, err := c.AuthenticateTokenCredentialRequest(audienceCtx, validRequest.DeepCopy())
@ -182,6 +184,50 @@ func TestAuthenticateTokenCredentialRequest(t *testing.T) {
require.Equal(t, []string{"test-group-1", "test-group-2"}, res.GetGroups())
require.Equal(t, map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, res.GetExtra())
})
t.Run("using a non-default API group suffix still performs the cache lookup using the pinniped.dev suffix", func(t *testing.T) {
authenticationGroupWithCustomSuffix := "authentication.concierge.custom-suffix.com"
validRequestForAlternateAPIGroup := loginapi.TokenCredentialRequest{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test-namespace",
},
Spec: loginapi.TokenCredentialRequestSpec{
Authenticator: corev1.TypedLocalObjectReference{
APIGroup: &authenticationGroupWithCustomSuffix,
Kind: "WebhookAuthenticator",
Name: "test-name",
},
Token: "test-token",
},
Status: loginapi.TokenCredentialRequestStatus{},
}
userInfo := user.DefaultInfo{
Name: "test-user",
UID: "test-uid",
Groups: []string{"test-group-1", "test-group-2"},
Extra: map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}},
}
c := mockCache(t, "custom-suffix.com", true, &authenticator.Response{User: &userInfo}, true, nil)
audienceCtx := authenticator.WithAudiences(context.Background(), authenticator.Audiences{"test-audience-1"})
res, err := c.AuthenticateTokenCredentialRequest(audienceCtx, validRequestForAlternateAPIGroup.DeepCopy())
require.NoError(t, err)
require.NotNil(t, res)
require.Equal(t, "test-user", res.GetName())
require.Equal(t, "test-uid", res.GetUID())
require.Equal(t, []string{"test-group-1", "test-group-2"}, res.GetGroups())
require.Equal(t, map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, res.GetExtra())
})
t.Run("using a non-default API group suffix and the incoming request mentions a different API group, results in no such authenticator", func(t *testing.T) {
c := mockCache(t, "custom-suffix.com", false, &authenticator.Response{User: &user.DefaultInfo{Name: "someone"}}, true, nil)
// Note that the validRequest.Spec.Authenticator.APIGroup value uses "pinniped.dev", not "custom-suffix.com"
res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy())
require.EqualError(t, err, "no such authenticator")
require.Nil(t, res)
})
}
type audienceFreeContext struct{}

2
internal/controller/authenticator/cachecleaner/cachecleaner_test.go
View File

@ -153,7 +153,7 @@ func TestController(t *testing.T) {
fakeClient := pinnipedfake.NewSimpleClientset(tt.objects...)
informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0)
cache := authncache.New()
cache := authncache.New("pinniped.dev")
if tt.initialCache != nil {
tt.initialCache(t, cache)
}

2
internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go
View File

@ -327,7 +327,7 @@ func TestController(t *testing.T) {
fakeClient := pinnipedfake.NewSimpleClientset(tt.jwtAuthenticators...)
informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0)
cache := authncache.New()
cache := authncache.New("pinniped.dev")
testLog := testlogger.New(t)
if tt.cache != nil {

2
internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go
View File

@ -90,7 +90,7 @@ func TestController(t *testing.T) {
fakeClient := pinnipedfake.NewSimpleClientset(tt.webhooks...)
informers := pinnipedinformers.NewSharedInformerFactory(fakeClient, 0)
cache := authncache.New()
cache := authncache.New("pinniped.dev")
testLog := testlogger.New(t)
controller := New(cache, informers.Authentication().V1alpha1().WebhookAuthenticators(), testLog)

5
internal/groupsuffix/groupsuffix.go
View File

@ -63,7 +63,7 @@ func New(apiGroupSuffix string) kubeclient.Middleware {
kubeclient.MiddlewareFunc(func(_ context.Context, rt kubeclient.RoundTrip) {
// always unreplace owner refs with apiGroupSuffix because we can consume those objects across all verbs
rt.MutateResponse(mutateOwnerRefs(unreplace, apiGroupSuffix))
rt.MutateResponse(mutateOwnerRefs(Unreplace, apiGroupSuffix))
}),
}
}
@ -115,7 +115,8 @@ func Replace(baseAPIGroup, apiGroupSuffix string) (string, bool) {
return strings.TrimSuffix(baseAPIGroup, pinnipedDefaultSuffix) + apiGroupSuffix, true
}
func unreplace(baseAPIGroup, apiGroupSuffix string) (string, bool) {
// Unreplace is like performing an undo of Replace().
func Unreplace(baseAPIGroup, apiGroupSuffix string) (string, bool) {
if !strings.HasSuffix(baseAPIGroup, "."+apiGroupSuffix) {
return "", false
}

27
internal/groupsuffix/groupsuffix_test.go
View File

@ -441,10 +441,12 @@ func TestMiddlware(t *testing.T) {
}
func TestReplaceError(t *testing.T) {
_, ok := Replace("bad-suffix-that-doesnt-end-in-pinniped-dot-dev", "shouldnt-matter.com")
s, ok := Replace("bad-suffix-that-doesnt-end-in-pinniped-dot-dev", "shouldnt-matter.com")
require.Equal(t, "", s)
require.False(t, ok)
_, ok = Replace("bad-suffix-that-end-in.prefixed-pinniped.dev", "shouldnt-matter.com")
s, ok = Replace("bad-suffix-that-end-in.prefixed-pinniped.dev", "shouldnt-matter.com")
require.Equal(t, "", s)
require.False(t, ok)
}
@ -452,6 +454,27 @@ func TestReplaceSuffix(t *testing.T) {
s, ok := Replace("something.pinniped.dev.something-else.pinniped.dev", "tuna.io")
require.Equal(t, "something.pinniped.dev.something-else.tuna.io", s)
require.True(t, ok)
// When the replace wasn't actually needed, it still returns true.
s, ok = Unreplace("something.pinniped.dev", "pinniped.dev")
require.Equal(t, "something.pinniped.dev", s)
require.True(t, ok)
}
func TestUnreplaceSuffix(t *testing.T) {
s, ok := Unreplace("something.pinniped.dev.something-else.tuna.io", "tuna.io")
require.Equal(t, "something.pinniped.dev.something-else.pinniped.dev", s)
require.True(t, ok)
// When the unreplace wasn't actually needed, it still returns true.
s, ok = Unreplace("something.pinniped.dev", "pinniped.dev")
require.Equal(t, "something.pinniped.dev", s)
require.True(t, ok)
// When the unreplace was needed but did not work, return false.
s, ok = Unreplace("something.pinniped.dev.something-else.tuna.io", "salmon.io")
require.Equal(t, "", s)
require.False(t, ok)
}
func TestValidate(t *testing.T) {

33
pkg/conciergeclient/conciergeclient.go
View File

@ -12,7 +12,7 @@ import (
"net/url"
"strings"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1"
"k8s.io/client-go/tools/clientcmd"
@ -34,11 +34,12 @@ type Option func(*Client) error
// Client is a configuration for talking to the Pinniped concierge.
type Client struct {
namespace string
authenticator *corev1.TypedLocalObjectReference
caBundle string
endpoint *url.URL
apiGroupSuffix string
namespace string
authenticatorName string
authenticatorKind string
caBundle string
endpoint *url.URL
apiGroupSuffix string
}
// WithNamespace configures the namespace where the TokenCredentialRequest is to be sent.
@ -55,18 +56,15 @@ func WithAuthenticator(authType, authName string) Option {
if authName == "" {
return fmt.Errorf("authenticator name must not be empty")
}
authenticator := corev1.TypedLocalObjectReference{Name: authName}
c.authenticatorName = authName
switch strings.ToLower(authType) {
case "webhook":
authenticator.APIGroup = &auth1alpha1.SchemeGroupVersion.Group
authenticator.Kind = "WebhookAuthenticator"
c.authenticatorKind = "WebhookAuthenticator"
case "jwt":
authenticator.APIGroup = &auth1alpha1.SchemeGroupVersion.Group
authenticator.Kind = "JWTAuthenticator"
c.authenticatorKind = "JWTAuthenticator"
default:
return fmt.Errorf(`invalid authenticator type: %q, supported values are "webhook" and "jwt"`, authType)
}
c.authenticator = &authenticator
return nil
}
}
@ -133,7 +131,7 @@ func New(opts ...Option) (*Client, error) {
return nil, err
}
}
if c.authenticator == nil {
if c.authenticatorName == "" {
return nil, fmt.Errorf("WithAuthenticator must be specified")
}
if c.endpoint == nil {
@ -180,13 +178,18 @@ func (c *Client) ExchangeToken(ctx context.Context, token string) (*clientauthen
if err != nil {
return nil, err
}
replacedAPIGroupName, _ := groupsuffix.Replace(auth1alpha1.SchemeGroupVersion.Group, c.apiGroupSuffix)
resp, err := clientset.LoginV1alpha1().TokenCredentialRequests(c.namespace).Create(ctx, &loginv1alpha1.TokenCredentialRequest{
ObjectMeta: metav1.ObjectMeta{
Namespace: c.namespace,
},
Spec: loginv1alpha1.TokenCredentialRequestSpec{
Token: token,
Authenticator: *c.authenticator,
Token: token,
Authenticator: v1.TypedLocalObjectReference{
APIGroup: &replacedAPIGroupName,
Kind: c.authenticatorKind,
Name: c.authenticatorName,
},
},
}, metav1.CreateOptions{})
if err != nil {

117
pkg/conciergeclient/conciergeclient_test.go
View File

@ -21,6 +21,7 @@ import (
loginv1alpha1 "go.pinniped.dev/generated/1.20/apis/concierge/login/v1alpha1"
"go.pinniped.dev/internal/certauthority"
"go.pinniped.dev/internal/here"
"go.pinniped.dev/internal/testutil"
)
@ -220,47 +221,7 @@ func TestExchangeToken(t *testing.T) {
t.Parallel()
expires := metav1.NewTime(time.Now().Truncate(time.Second))
// Start a test server that returns successfully and asserts various properties of the request.
caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, http.MethodPost, r.Method)
require.Equal(t, "/apis/login.concierge.pinniped.dev/v1alpha1/namespaces/test-namespace/tokencredentialrequests", r.URL.Path)
require.Equal(t, "application/json", r.Header.Get("content-type"))
body, err := ioutil.ReadAll(r.Body)
require.NoError(t, err)
require.JSONEq(t,
`{
"kind": "TokenCredentialRequest",
"apiVersion": "login.concierge.pinniped.dev/v1alpha1",
"metadata": {
"creationTimestamp": null,
"namespace": "test-namespace"
},
"spec": {
"token": "test-token",
"authenticator": {
"apiGroup": "authentication.concierge.pinniped.dev",
"kind": "WebhookAuthenticator",
"name": "test-webhook"
}
},
"status": {}
}`,
string(body),
)
w.Header().Set("content-type", "application/json")
_ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{
TypeMeta: metav1.TypeMeta{APIVersion: "login.concierge.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"},
Status: loginv1alpha1.TokenCredentialRequestStatus{
Credential: &loginv1alpha1.ClusterCredential{
ExpirationTimestamp: expires,
ClientCertificateData: "test-certificate",
ClientKeyData: "test-key",
},
},
})
})
caBundle, endpoint := runFakeServer(t, expires, "pinniped.dev")
client, err := New(WithNamespace("test-namespace"), WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("webhook", "test-webhook"))
require.NoError(t, err)
@ -279,4 +240,78 @@ func TestExchangeToken(t *testing.T) {
},
}, got)
})
t.Run("changing the API group suffix for the client sends the custom suffix on the CredentialRequest's APIGroup and on its spec.Authenticator.APIGroup", func(t *testing.T) {
t.Parallel()
expires := metav1.NewTime(time.Now().Truncate(time.Second))
caBundle, endpoint := runFakeServer(t, expires, "suffix.com")
client, err := New(WithAPIGroupSuffix("suffix.com"), WithNamespace("test-namespace"), WithEndpoint(endpoint), WithCABundle(caBundle), WithAuthenticator("webhook", "test-webhook"))
require.NoError(t, err)
got, err := client.ExchangeToken(ctx, "test-token")
require.NoError(t, err)
require.Equal(t, &clientauthenticationv1beta1.ExecCredential{
TypeMeta: metav1.TypeMeta{
Kind: "ExecCredential",
APIVersion: "client.authentication.k8s.io/v1beta1",
},
Status: &clientauthenticationv1beta1.ExecCredentialStatus{
ClientCertificateData: "test-certificate",
ClientKeyData: "test-key",
ExpirationTimestamp: &expires,
},
}, got)
})
}
// Start a test server that returns successfully and asserts various properties of the request.
func runFakeServer(t *testing.T, expires metav1.Time, pinnipedAPIGroupSuffix string) (string, string) {
caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, http.MethodPost, r.Method)
require.Equal(t,
fmt.Sprintf("/apis/login.concierge.%s/v1alpha1/namespaces/test-namespace/tokencredentialrequests", pinnipedAPIGroupSuffix),
r.URL.Path)
require.Equal(t, "application/json", r.Header.Get("content-type"))
body, err := ioutil.ReadAll(r.Body)
require.NoError(t, err)
require.JSONEq(t, here.Docf(
`{
"kind": "TokenCredentialRequest",
"apiVersion": "login.concierge.%s/v1alpha1",
"metadata": {
"creationTimestamp": null,
"namespace": "test-namespace"
},
"spec": {
"token": "test-token",
"authenticator": {
"apiGroup": "authentication.concierge.%s",
"kind": "WebhookAuthenticator",
"name": "test-webhook"
}
},
"status": {}
}`, pinnipedAPIGroupSuffix, pinnipedAPIGroupSuffix),
string(body),
)
w.Header().Set("content-type", "application/json")
_ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{
TypeMeta: metav1.TypeMeta{
APIVersion: fmt.Sprintf("login.concierge.%s/v1alpha1", pinnipedAPIGroupSuffix),
Kind: "TokenCredentialRequest",
},
Status: loginv1alpha1.TokenCredentialRequestStatus{
Credential: &loginv1alpha1.ClusterCredential{
ExpirationTimestamp: expires,
ClientCertificateData: "test-certificate",
ClientKeyData: "test-key",
},
},
})
})
return caBundle, endpoint
}

10
test/library/client.go
View File

@ -180,8 +180,11 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
require.NoErrorf(t, err, "could not cleanup test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name)
})
apiGroup, replacedSuffix := groupsuffix.Replace(auth1alpha1.SchemeGroupVersion.Group, testEnv.APIGroupSuffix)
require.True(t, replacedSuffix)
return corev1.TypedLocalObjectReference{
APIGroup: &auth1alpha1.SchemeGroupVersion.Group,
APIGroup: &apiGroup,
Kind: "WebhookAuthenticator",
Name: webhook.Name,
}
@ -250,8 +253,11 @@ func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T, spec auth1alp
require.NoErrorf(t, err, "could not cleanup test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name)
})
apiGroup, replacedSuffix := groupsuffix.Replace(auth1alpha1.SchemeGroupVersion.Group, testEnv.APIGroupSuffix)
require.True(t, replacedSuffix)
return corev1.TypedLocalObjectReference{
APIGroup: &auth1alpha1.SchemeGroupVersion.Group,
APIGroup: &apiGroup,
Kind: "JWTAuthenticator",
Name: jwtAuthenticator.Name,
}