diff --git a/pkg/registry/certificates/registry.go b/pkg/registry/certificates/registry.go index 59142125c64..e812ef00077 100644 --- a/pkg/registry/certificates/registry.go +++ b/pkg/registry/certificates/registry.go @@ -54,17 +54,6 @@ func (s *storage) ListCSRs(ctx api.Context, options *api.ListOptions) (*certific } func (s *storage) CreateCSR(ctx api.Context, csr *certificates.CertificateSigningRequest) error { - // Inject user.Info from request context if available and no - // information was supplied. Don't smash existing user information - // since it should be possible for a broadly-scoped user to request a - // certificate on behalf of a more limited user. - if user, ok := api.UserFrom(ctx); ok { - if csr.Spec.Username == "" { - csr.Spec.Username = user.GetName() - csr.Spec.UID = user.GetUID() - csr.Spec.Groups = user.GetGroups() - } - } _, err := s.Create(ctx, csr) return err } diff --git a/pkg/registry/certificates/strategy.go b/pkg/registry/certificates/strategy.go index 35726dc325d..03e4659aa2d 100644 --- a/pkg/registry/certificates/strategy.go +++ b/pkg/registry/certificates/strategy.go @@ -50,13 +50,21 @@ func (csrStrategy) AllowCreateOnUpdate() bool { } // PrepareForCreate clears fields that are not allowed to be set by end users -// on creation. Users cannot create any derived information, but we expect -// information about the requesting user to be injected by the registry -// interface. Clear everything else. -// TODO: check these ordering assumptions. better way to inject user info? +// on creation. func (csrStrategy) PrepareForCreate(ctx api.Context, obj runtime.Object) { csr := obj.(*certificates.CertificateSigningRequest) + // Clear any user-specified info + csr.Spec.Username = "" + csr.Spec.UID = "" + csr.Spec.Groups = nil + // Inject user.Info from request context + if user, ok := api.UserFrom(ctx); ok { + csr.Spec.Username = user.GetName() + csr.Spec.UID = user.GetUID() + csr.Spec.Groups = user.GetGroups() + } + // Be explicit that users cannot create pre-approved certificate requests. csr.Status = certificates.CertificateSigningRequestStatus{} csr.Status.Conditions = []certificates.CertificateSigningRequestCondition{} diff --git a/pkg/registry/certificates/strategy_test.go b/pkg/registry/certificates/strategy_test.go new file mode 100644 index 00000000000..182fa015ec2 --- /dev/null +++ b/pkg/registry/certificates/strategy_test.go @@ -0,0 +1,121 @@ +/* +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 certificates + +import ( + "reflect" + "testing" + + "k8s.io/kubernetes/pkg/api" + certapi "k8s.io/kubernetes/pkg/apis/certificates" + "k8s.io/kubernetes/pkg/auth/user" + "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/diff" +) + +func TestStrategyCreate(t *testing.T) { + tests := map[string]struct { + ctx api.Context + obj runtime.Object + expectedObj runtime.Object + }{ + "no user in context, no user in obj": { + ctx: api.NewContext(), + obj: &certapi.CertificateSigningRequest{}, + expectedObj: &certapi.CertificateSigningRequest{ + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }, + }, + "user in context, no user in obj": { + ctx: api.WithUser( + api.NewContext(), + &user.DefaultInfo{ + Name: "bob", + UID: "123", + Groups: []string{"group1"}, + Extra: map[string][]string{"foo": {"bar"}}, + }, + ), + obj: &certapi.CertificateSigningRequest{}, + expectedObj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + Username: "bob", + UID: "123", + Groups: []string{"group1"}, + }, + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }, + }, + "no user in context, user in obj": { + ctx: api.NewContext(), + obj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + Username: "bob", + UID: "123", + Groups: []string{"group1"}, + }, + }, + expectedObj: &certapi.CertificateSigningRequest{ + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }, + }, + "user in context, user in obj": { + ctx: api.WithUser( + api.NewContext(), + &user.DefaultInfo{ + Name: "alice", + UID: "234", + }, + ), + obj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + Username: "bob", + UID: "123", + Groups: []string{"group1"}, + }, + }, + expectedObj: &certapi.CertificateSigningRequest{ + Spec: certapi.CertificateSigningRequestSpec{ + Username: "alice", + UID: "234", + Groups: nil, + }, + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }, + }, + "pre-approved status": { + ctx: api.NewContext(), + obj: &certapi.CertificateSigningRequest{ + Status: certapi.CertificateSigningRequestStatus{ + Conditions: []certapi.CertificateSigningRequestCondition{ + {Type: certapi.CertificateApproved}, + }, + }, + }, + expectedObj: &certapi.CertificateSigningRequest{ + Status: certapi.CertificateSigningRequestStatus{Conditions: []certapi.CertificateSigningRequestCondition{}}, + }}, + } + + for k, tc := range tests { + obj := tc.obj + Strategy.PrepareForCreate(tc.ctx, obj) + if !reflect.DeepEqual(obj, tc.expectedObj) { + t.Errorf("%s: object diff: %s", k, diff.ObjectDiff(obj, tc.expectedObj)) + } + } +}