Convert user/group * to match authenticated users only in ABAC

hack/.linted_packages

@@ -75,6 +75,7 @@ pkg/api/v1
 pkg/api/v1/service
 pkg/apimachinery
 pkg/apis/abac/v0
+pkg/apis/abac/v1beta1
 pkg/apis/apps/install
 pkg/apis/authentication.k8s.io/install
 pkg/apis/authentication/install

pkg/apis/abac/v0/BUILD

@@ -19,6 +19,7 @@ go_library(
     deps = [
         "//pkg/apis/abac:go_default_library",
         "//pkg/apis/meta/v1:go_default_library",
+        "//pkg/auth/user:go_default_library",
         "//pkg/conversion:go_default_library",
         "//pkg/runtime:go_default_library",
         "//pkg/runtime/schema:go_default_library",
@@ -32,5 +33,6 @@ go_test(
     deps = [
         "//pkg/apis/abac:go_default_library",
         "//pkg/apis/abac/v0:go_default_library",
+        "//pkg/auth/user:go_default_library",
     ],
 )

pkg/apis/abac/v0/conversion.go

@@ -18,6 +18,7 @@ package v0
 
 import (
 	api "k8s.io/kubernetes/pkg/apis/abac"
+	"k8s.io/kubernetes/pkg/auth/user"
 	"k8s.io/kubernetes/pkg/conversion"
 	"k8s.io/kubernetes/pkg/runtime"
 )
@@ -32,9 +33,14 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 			out.Spec.Resource = in.Resource
 			out.Spec.Readonly = in.Readonly
 
-			// In v0, unspecified user and group matches all subjects
+			// In v0, unspecified user and group matches all authenticated subjects
 			if len(in.User) == 0 && len(in.Group) == 0 {
-				out.Spec.User = "*"
+				out.Spec.Group = user.AllAuthenticated
+			}
+			// In v0, user or group of * matches all authenticated subjects
+			if in.User == "*" || in.Group == "*" {
+				out.Spec.Group = user.AllAuthenticated
+				out.Spec.User = ""
 			}
 
 			// In v0, leaving namespace empty matches all namespaces

pkg/apis/abac/v0/conversion_test.go

@@ -22,9 +22,10 @@ import (
 
 	api "k8s.io/kubernetes/pkg/apis/abac"
 	"k8s.io/kubernetes/pkg/apis/abac/v0"
+	"k8s.io/kubernetes/pkg/auth/user"
 )
 
-func TestConversion(t *testing.T) {
+func TestV0Conversion(t *testing.T) {
 	testcases := map[string]struct {
 		old      *v0.Policy
 		expected *api.Policy
@@ -32,7 +33,7 @@ func TestConversion(t *testing.T) {
 		// a completely empty policy rule allows everything to all users
 		"empty": {
 			old:      &v0.Policy{},
-			expected: &api.Policy{Spec: api.PolicySpec{User: "*", Readonly: false, NonResourcePath: "*", Namespace: "*", Resource: "*", APIGroup: "*"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated, Readonly: false, NonResourcePath: "*", Namespace: "*", Resource: "*", APIGroup: "*"}},
 		},
 
 		// specifying a user is preserved
@@ -47,22 +48,32 @@ func TestConversion(t *testing.T) {
 			expected: &api.Policy{Spec: api.PolicySpec{Group: "mygroup", Readonly: false, NonResourcePath: "*", Namespace: "*", Resource: "*", APIGroup: "*"}},
 		},
 
+		// specifying * for user or group maps to all authenticated subjects
+		"* user": {
+			old:      &v0.Policy{User: "*"},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated, Readonly: false, NonResourcePath: "*", Namespace: "*", Resource: "*", APIGroup: "*"}},
+		},
+		"* group": {
+			old:      &v0.Policy{Group: "*"},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated, Readonly: false, NonResourcePath: "*", Namespace: "*", Resource: "*", APIGroup: "*"}},
+		},
+
 		// specifying a namespace removes the * match on non-resource path
 		"namespace": {
 			old:      &v0.Policy{Namespace: "myns"},
-			expected: &api.Policy{Spec: api.PolicySpec{User: "*", Readonly: false, NonResourcePath: "", Namespace: "myns", Resource: "*", APIGroup: "*"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated, Readonly: false, NonResourcePath: "", Namespace: "myns", Resource: "*", APIGroup: "*"}},
 		},
 
 		// specifying a resource removes the * match on non-resource path
 		"resource": {
 			old:      &v0.Policy{Resource: "myresource"},
-			expected: &api.Policy{Spec: api.PolicySpec{User: "*", Readonly: false, NonResourcePath: "", Namespace: "*", Resource: "myresource", APIGroup: "*"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated, Readonly: false, NonResourcePath: "", Namespace: "*", Resource: "myresource", APIGroup: "*"}},
 		},
 
 		// specifying a namespace+resource removes the * match on non-resource path
 		"namespace+resource": {
 			old:      &v0.Policy{Namespace: "myns", Resource: "myresource"},
-			expected: &api.Policy{Spec: api.PolicySpec{User: "*", Readonly: false, NonResourcePath: "", Namespace: "myns", Resource: "myresource", APIGroup: "*"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated, Readonly: false, NonResourcePath: "", Namespace: "myns", Resource: "myresource", APIGroup: "*"}},
 		},
 	}
 	for k, tc := range testcases {

pkg/apis/abac/v1beta1/BUILD

@@ -5,19 +5,37 @@ licenses(["notice"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
     name = "go_default_library",
     srcs = [
+        "conversion.go",
+        "doc.go",
         "register.go",
         "types.go",
+        "zz_generated.conversion.go",
+        "zz_generated.deepcopy.go",
     ],
     tags = ["automanaged"],
     deps = [
         "//pkg/apis/abac:go_default_library",
         "//pkg/apis/meta/v1:go_default_library",
+        "//pkg/auth/user:go_default_library",
+        "//pkg/conversion:go_default_library",
         "//pkg/runtime:go_default_library",
         "//pkg/runtime/schema:go_default_library",
     ],
 )
+
+go_test(
+    name = "go_default_xtest",
+    srcs = ["conversion_test.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//pkg/apis/abac:go_default_library",
+        "//pkg/apis/abac/v1beta1:go_default_library",
+        "//pkg/auth/user:go_default_library",
+    ],
+)

pkg/apis/abac/v1beta1/conversion.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+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 v1beta1
+
+import (
+	api "k8s.io/kubernetes/pkg/apis/abac"
+	"k8s.io/kubernetes/pkg/auth/user"
+	"k8s.io/kubernetes/pkg/conversion"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+func addConversionFuncs(scheme *runtime.Scheme) error {
+	return scheme.AddConversionFuncs(
+		func(in *Policy, out *api.Policy, s conversion.Scope) error {
+			// Begin by copying all fields
+			if err := autoConvert_v1beta1_Policy_To_abac_Policy(in, out, s); err != nil {
+				return err
+			}
+
+			// In v1beta1, * user or group maps to all authenticated subjects
+			if in.Spec.User == "*" || in.Spec.Group == "*" {
+				out.Spec.Group = user.AllAuthenticated
+				out.Spec.User = ""
+			}
+
+			return nil
+		},
+	)
+}

pkg/apis/abac/v1beta1/conversion_test.go

+/*
+Copyright 2016 The Kubernetes Authors.
+
+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 v1beta1_test
+
+import (
+	"reflect"
+	"testing"
+
+	api "k8s.io/kubernetes/pkg/apis/abac"
+	"k8s.io/kubernetes/pkg/apis/abac/v1beta1"
+	"k8s.io/kubernetes/pkg/auth/user"
+)
+
+func TestV1Beta1Conversion(t *testing.T) {
+	testcases := map[string]struct {
+		old      *v1beta1.Policy
+		expected *api.Policy
+	}{
+		// specifying a user is preserved
+		"user": {
+			old:      &v1beta1.Policy{Spec: v1beta1.PolicySpec{User: "bob"}},
+			expected: &api.Policy{Spec: api.PolicySpec{User: "bob"}},
+		},
+
+		// specifying a group is preserved
+		"group": {
+			old:      &v1beta1.Policy{Spec: v1beta1.PolicySpec{Group: "mygroup"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: "mygroup"}},
+		},
+
+		// specifying * for user or group maps to all authenticated subjects
+		"* user": {
+			old:      &v1beta1.Policy{Spec: v1beta1.PolicySpec{User: "*"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated}},
+		},
+		"* group": {
+			old:      &v1beta1.Policy{Spec: v1beta1.PolicySpec{Group: "*"}},
+			expected: &api.Policy{Spec: api.PolicySpec{Group: user.AllAuthenticated}},
+		},
+	}
+	for k, tc := range testcases {
+		internal := &api.Policy{}
+		if err := api.Scheme.Convert(tc.old, internal, nil); err != nil {
+			t.Errorf("%s: unexpected error: %v", k, err)
+		}
+		if !reflect.DeepEqual(internal, tc.expected) {
+			t.Errorf("%s: expected\n\t%#v, got \n\t%#v", k, tc.expected, internal)
+		}
+	}
+}

pkg/apis/abac/v1beta1/register.go

@@ -33,10 +33,14 @@ func init() {
 		// Programmer error.
 		panic(err)
 	}
+	if err := addConversionFuncs(api.Scheme); err != nil {
+		// Programmer error.
+		panic(err)
+	}
 }
 
 var (
-	SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)
+	SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes, addConversionFuncs)
 	AddToScheme   = SchemeBuilder.AddToScheme
 )
 

pkg/auth/authorizer/abac/example_policy_file.jsonl

-{"apiVersion": "abac.authorization.kubernetes.io/v1beta1", "kind": "Policy", "spec": {"user":"*",         "nonResourcePath": "*", "readonly": true}}
+{"apiVersion": "abac.authorization.kubernetes.io/v1beta1", "kind": "Policy", "spec": {"group":"system:authenticated",  "nonResourcePath": "*", "readonly": true}}
+{"apiVersion": "abac.authorization.kubernetes.io/v1beta1", "kind": "Policy", "spec": {"user":"system:unauthenticated", "nonResourcePath": "*", "readonly": true}}
 {"apiVersion": "abac.authorization.kubernetes.io/v1beta1", "kind": "Policy", "spec": {"user":"admin",     "namespace": "*",              "resource": "*",         "apiGroup": "*"                   }}
 {"apiVersion": "abac.authorization.kubernetes.io/v1beta1", "kind": "Policy", "spec": {"user":"scheduler", "namespace": "*",              "resource": "pods",                       "readonly": true }}
 {"apiVersion": "abac.authorization.kubernetes.io/v1beta1", "kind": "Policy", "spec": {"user":"scheduler", "namespace": "*",              "resource": "bindings"                                     }}

test/integration/auth/auth_test.go

@@ -46,6 +46,7 @@ import (
 	"k8s.io/kubernetes/pkg/auth/authenticator/bearertoken"
 	"k8s.io/kubernetes/pkg/auth/authorizer"
 	"k8s.io/kubernetes/pkg/auth/authorizer/abac"
+	"k8s.io/kubernetes/pkg/auth/group"
 	"k8s.io/kubernetes/pkg/auth/user"
 	"k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api/v1"
 	apiserverauthorizer "k8s.io/kubernetes/pkg/genericapiserver/authorizer"
@@ -67,7 +68,7 @@ func getTestTokenAuth() authenticator.Request {
 	tokenAuthenticator := tokentest.New()
 	tokenAuthenticator.Tokens[AliceToken] = &user.DefaultInfo{Name: "alice", UID: "1"}
 	tokenAuthenticator.Tokens[BobToken] = &user.DefaultInfo{Name: "bob", UID: "2"}
-	return bearertoken.New(tokenAuthenticator)
+	return group.NewGroupAdder(bearertoken.New(tokenAuthenticator), []string{user.AllAuthenticated})
 }
 
 func getTestWebhookTokenAuth(serverURL string) (authenticator.Request, error) {

test/test_owners.csv

@@ -559,6 +559,7 @@ k8s.io/kubernetes/pkg/apimachinery,gmarek,1
 k8s.io/kubernetes/pkg/apimachinery/announced,kargakis,1
 k8s.io/kubernetes/pkg/apimachinery/registered,jlowdermilk,1
 k8s.io/kubernetes/pkg/apis/abac/v0,liggitt,0
+k8s.io/kubernetes/pkg/apis/abac/v1beta1,liggitt,0
 k8s.io/kubernetes/pkg/apis/apps/validation,derekwaynecarr,1
 k8s.io/kubernetes/pkg/apis/authorization/validation,erictune,0
 k8s.io/kubernetes/pkg/apis/autoscaling/v1,yarntime,0