Use custom suffix in Spec.Authenticator.APIGroup of TokenCredentialRequest

When the Pinniped server has been installed with the `api_group_suffix`
option, for example using `mysuffix.com`, then clients who would like to
submit a `TokenCredentialRequest` to the server should set the
`Spec.Authenticator.APIGroup` field as `authentication.concierge.mysuffix.com`.

This makes more sense from the client's point of view than using the
default `authentication.concierge.pinniped.dev` because
`authentication.concierge.mysuffix.com` is the name of the API group
that they can observe their cluster and `authentication.concierge.pinniped.dev`
does not exist as an API group on their cluster.

This commit includes both the client and server-side changes to make
this work, as well as integration test updates.

Co-authored-by: Andrew Keesler <akeesler@vmware.com>
Co-authored-by: Ryan Richard <richardry@vmware.com>
Co-authored-by: Margo Crawford <margaretc@vmware.com>
This commit is contained in:
Ryan Richard
2021-02-03 15:49:15 -08:00
co-authored by Andrew Keesler Margo Crawford
parent 26922307ad
commit 288d9c999e
11 changed files with 203 additions and 82 deletions
@@ -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)
@@ -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{}
@@ -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)
}
@@ -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 {
@@ -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)