Merge pull request #8607 from liggitt/serviceaccount_groups

pkg/controller/serviceaccount/jwt.go

@@ -22,7 +22,6 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"strings"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/authenticator"
@@ -33,9 +32,6 @@ import (
 )
 
 const (
-	ServiceAccountUsernamePrefix    = "system:serviceaccount"
-	ServiceAccountUsernameSeparator = ":"
-
 	Issuer = "kubernetes/serviceaccount"
 
 	SubjectClaim            = "sub"
@@ -76,26 +72,6 @@ func ReadPublicKey(file string) (*rsa.PublicKey, error) {
 	return jwt.ParseRSAPublicKeyFromPEM(data)
 }
 
-// MakeUsername generates a username from the given namespace and ServiceAccount name.
-// The resulting username can be passed to SplitUsername to extract the original namespace and ServiceAccount name.
-func MakeUsername(namespace, name string) string {
-	return strings.Join([]string{ServiceAccountUsernamePrefix, namespace, name}, ServiceAccountUsernameSeparator)
-}
-
-// SplitUsername returns the namespace and ServiceAccount name embedded in the given username,
-// or an error if the username is not a valid name produced by MakeUsername
-func SplitUsername(username string) (string, string, error) {
-	if !strings.HasPrefix(username, ServiceAccountUsernamePrefix+ServiceAccountUsernameSeparator) {
-		return "", "", fmt.Errorf("Username must be in the form %s", MakeUsername("namespace", "name"))
-	}
-	username = strings.TrimPrefix(username, ServiceAccountUsernamePrefix+ServiceAccountUsernameSeparator)
-	parts := strings.Split(username, ServiceAccountUsernameSeparator)
-	if len(parts) != 2 || len(parts[0]) == 0 || len(parts[1]) == 0 {
-		return "", "", fmt.Errorf("Username must be in the form %s", MakeUsername("namespace", "name"))
-	}
-	return parts[0], parts[1], nil
-}
-
 // JWTTokenGenerator returns a TokenGenerator that generates signed JWT tokens, using the given privateKey.
 // privateKey is a PEM-encoded byte array of a private RSA key.
 // JWTTokenAuthenticator()
@@ -113,7 +89,7 @@ func (j *jwtTokenGenerator) GenerateToken(serviceAccount api.ServiceAccount, sec
 	// Identify the issuer
 	token.Claims[IssuerClaim] = Issuer
 
-	// Username: `serviceaccount:<namespace>:<serviceaccount>`
+	// Username
 	token.Claims[SubjectClaim] = MakeUsername(serviceAccount.Namespace, serviceAccount.Name)
 
 	// Persist enough structured info for the authenticator to be able to look up the service account and secret
@@ -202,6 +178,11 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(token string) (user.Info, bool
 			return nil, false, errors.New("serviceAccountUID claim is missing")
 		}
 
+		subjectNamespace, subjectName, err := SplitUsername(sub)
+		if err != nil || subjectNamespace != namespace || subjectName != serviceAccountName {
+			return nil, false, errors.New("sub claim is invalid")
+		}
+
 		if j.lookup {
 			// Make sure token hasn't been invalidated by deletion of the secret
 			secret, err := j.getter.GetSecret(namespace, secretName)
@@ -226,11 +207,7 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(token string) (user.Info, bool
 			}
 		}
 
-		return &user.DefaultInfo{
-			Name:   sub,
-			UID:    serviceAccountUID,
-			Groups: []string{},
-		}, true, nil
+		return UserInfo(namespace, serviceAccountName, serviceAccountUID), true, nil
 	}
 
 	return nil, false, validationError

pkg/controller/serviceaccount/jwt_test.go

@@ -20,6 +20,7 @@ import (
 	"crypto/rsa"
 	"io/ioutil"
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -162,6 +163,7 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 		ExpectedOK       bool
 		ExpectedUserName string
 		ExpectedUserUID  string
+		ExpectedGroups   []string
 	}{
 		"no keys": {
 			Client:      nil,
@@ -182,6 +184,7 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 			ExpectedOK:       true,
 			ExpectedUserName: expectedUserName,
 			ExpectedUserUID:  expectedUserUID,
+			ExpectedGroups:   []string{"system:serviceaccounts", "system:serviceaccounts:test"},
 		},
 		"rotated keys": {
 			Client:           nil,
@@ -190,6 +193,7 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 			ExpectedOK:       true,
 			ExpectedUserName: expectedUserName,
 			ExpectedUserUID:  expectedUserUID,
+			ExpectedGroups:   []string{"system:serviceaccounts", "system:serviceaccounts:test"},
 		},
 		"valid lookup": {
 			Client:           testclient.NewSimpleFake(serviceAccount, secret),
@@ -198,6 +202,7 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 			ExpectedOK:       true,
 			ExpectedUserName: expectedUserName,
 			ExpectedUserUID:  expectedUserUID,
+			ExpectedGroups:   []string{"system:serviceaccounts", "system:serviceaccounts:test"},
 		},
 		"invalid secret lookup": {
 			Client:      testclient.NewSimpleFake(serviceAccount),
@@ -240,6 +245,10 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 			t.Errorf("%s: Expected userUID=%v, got %v", k, tc.ExpectedUserUID, user.GetUID())
 			continue
 		}
+		if !reflect.DeepEqual(user.GetGroups(), tc.ExpectedGroups) {
+			t.Errorf("%s: Expected groups=%v, got %v", k, tc.ExpectedGroups, user.GetGroups())
+			continue
+		}
 	}
 }
 

pkg/controller/serviceaccount/util.go

+/*
+Copyright 2014 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package serviceaccount
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user"
+)
+
+const (
+	ServiceAccountUsernamePrefix    = "system:serviceaccount:"
+	ServiceAccountUsernameSeparator = ":"
+	ServiceAccountGroupPrefix       = "system:serviceaccounts:"
+	AllServiceAccountsGroup         = "system:serviceaccounts"
+)
+
+// MakeUsername generates a username from the given namespace and ServiceAccount name.
+// The resulting username can be passed to SplitUsername to extract the original namespace and ServiceAccount name.
+func MakeUsername(namespace, name string) string {
+	return ServiceAccountUsernamePrefix + namespace + ServiceAccountUsernameSeparator + name
+}
+
+var invalidUsernameErr = fmt.Errorf("Username must be in the form %s", MakeUsername("namespace", "name"))
+
+// SplitUsername returns the namespace and ServiceAccount name embedded in the given username,
+// or an error if the username is not a valid name produced by MakeUsername
+func SplitUsername(username string) (string, string, error) {
+	if !strings.HasPrefix(username, ServiceAccountUsernamePrefix) {
+		return "", "", invalidUsernameErr
+	}
+	trimmed := strings.TrimPrefix(username, ServiceAccountUsernamePrefix)
+	parts := strings.Split(trimmed, ServiceAccountUsernameSeparator)
+	if len(parts) != 2 {
+		return "", "", invalidUsernameErr
+	}
+	namespace, name := parts[0], parts[1]
+	if ok, _ := validation.ValidateNamespaceName(namespace, false); !ok {
+		return "", "", invalidUsernameErr
+	}
+	if ok, _ := validation.ValidateServiceAccountName(name, false); !ok {
+		return "", "", invalidUsernameErr
+	}
+	return namespace, name, nil
+}
+
+// MakeGroupNames generates service account group names for the given namespace and ServiceAccount name
+func MakeGroupNames(namespace, name string) []string {
+	return []string{
+		AllServiceAccountsGroup,
+		MakeNamespaceGroupName(namespace),
+	}
+}
+
+// MakeNamespaceGroupName returns the name of the group all service accounts in the namespace are included in
+func MakeNamespaceGroupName(namespace string) string {
+	return ServiceAccountGroupPrefix + namespace
+}
+
+// UserInfo returns a user.Info interface for the given namespace, service account name and UID
+func UserInfo(namespace, name, uid string) user.Info {
+	return &user.DefaultInfo{
+		Name:   MakeUsername(namespace, name),
+		UID:    uid,
+		Groups: MakeGroupNames(namespace, name),
+	}
+}

pkg/controller/serviceaccount/util_test.go

+/*
+Copyright 2014 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package serviceaccount
+
+import "testing"
+
+func TestMakeUsername(t *testing.T) {
+
+	testCases := map[string]struct {
+		Namespace   string
+		Name        string
+		ExpectedErr bool
+	}{
+		"valid": {
+			Namespace:   "foo",
+			Name:        "bar",
+			ExpectedErr: false,
+		},
+		"empty": {
+			ExpectedErr: true,
+		},
+		"empty namespace": {
+			Namespace:   "",
+			Name:        "foo",
+			ExpectedErr: true,
+		},
+		"empty name": {
+			Namespace:   "foo",
+			Name:        "",
+			ExpectedErr: true,
+		},
+		"extra segments": {
+			Namespace:   "foo",
+			Name:        "bar:baz",
+			ExpectedErr: true,
+		},
+		"invalid chars in namespace": {
+			Namespace:   "foo ",
+			Name:        "bar",
+			ExpectedErr: true,
+		},
+		"invalid chars in name": {
+			Namespace:   "foo",
+			Name:        "bar ",
+			ExpectedErr: true,
+		},
+	}
+
+	for k, tc := range testCases {
+		username := MakeUsername(tc.Namespace, tc.Name)
+
+		namespace, name, err := SplitUsername(username)
+		if (err != nil) != tc.ExpectedErr {
+			t.Errorf("%s: Expected error=%v, got %v", k, tc.ExpectedErr, err)
+			continue
+		}
+		if err != nil {
+			continue
+		}
+
+		if namespace != tc.Namespace {
+			t.Errorf("%s: Expected namespace %q, got %q", k, tc.Namespace, namespace)
+		}
+		if name != tc.Name {
+			t.Errorf("%s: Expected name %q, got %q", k, tc.Name, name)
+		}
+	}
+}