[WIP, DO NOT MERGE] Ensure user has access to image repo in tag

pkg/authorization/registry/subjectaccessreview/registry.go

@@ -0,0 +1,31 @@
+package subjectaccessreview
+
+import (
+	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	api "github.com/openshift/origin/pkg/authorization/api"
+)
+
+type Registry interface {
+	CreateSubjectAccessReview(ctx kapi.Context, subjectAccessReview *api.SubjectAccessReview) (*api.SubjectAccessReviewResponse, error)
+}
+
+type Storage interface {
+	Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, error)
+}
+
+type storage struct {
+	Storage
+}
+
+func NewRegistry(s Storage) Registry {
+	return &storage{s}
+}
+
+func (s *storage) CreateSubjectAccessReview(ctx kapi.Context, subjectAccessReview *api.SubjectAccessReview) (*api.SubjectAccessReviewResponse, error) {
+	obj, err := s.Create(ctx, subjectAccessReview)
+	if err != nil {
+		return nil, err
+	}
+	return obj.(*api.SubjectAccessReviewResponse), nil
+}

pkg/authorization/registry/subjectaccessreview/rest.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
-	"github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 
@@ -19,7 +18,7 @@ type REST struct {
 }
 
 // NewREST creates a new REST for policies.
-func NewREST(authorizer authorizer.Authorizer) apiserver.RESTStorage {
+func NewREST(authorizer authorizer.Authorizer) *REST {
 	return &REST{authorizer}
 }
 

pkg/cmd/server/origin/master.go

@@ -86,7 +86,7 @@ import (
 	resourceaccessreviewregistry "github.com/openshift/origin/pkg/authorization/registry/resourceaccessreview"
 	roleregistry "github.com/openshift/origin/pkg/authorization/registry/role"
 	rolebindingregistry "github.com/openshift/origin/pkg/authorization/registry/rolebinding"
-	subjectaccessreviewregistry "github.com/openshift/origin/pkg/authorization/registry/subjectaccessreview"
+	"github.com/openshift/origin/pkg/authorization/registry/subjectaccessreview"
 	"github.com/openshift/origin/pkg/cmd/server/admin"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
 	routeplugin "github.com/openshift/origin/plugins/route/allocation/simple"
@@ -201,9 +201,12 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin
 	oauthEtcd := oauthetcd.New(c.EtcdHelper)
 	authorizationEtcd := authorizationetcd.New(c.EtcdHelper)
 
+	subjectAccessReviewStorage := subjectaccessreview.NewREST(c.Authorizer)
+	subjectAccessReviewRegistry := subjectaccessreview.NewRegistry(subjectAccessReviewStorage)
+
 	imageStorage := imageetcd.NewREST(c.EtcdHelper)
 	imageRegistry := image.NewRegistry(imageStorage)
-	imageRepositoryStorage, imageRepositoryStatus := imagerepositoryetcd.NewREST(c.EtcdHelper, imagerepository.DefaultRegistryFunc(defaultRegistryFunc))
+	imageRepositoryStorage, imageRepositoryStatus := imagerepositoryetcd.NewREST(c.EtcdHelper, imagerepository.DefaultRegistryFunc(defaultRegistryFunc), subjectAccessReviewRegistry)
 	imageRepositoryRegistry := imagerepository.NewRegistry(imageRepositoryStorage, imageRepositoryStatus)
 	imageRepositoryMappingStorage := imagerepositorymapping.NewREST(imageRegistry, imageRepositoryRegistry)
 	imageRepositoryTagStorage := imagerepositorytag.NewREST(imageRegistry, imageRepositoryRegistry)
@@ -268,7 +271,7 @@ func (c *MasterConfig) InstallProtectedAPI(container *restful.Container) []strin
 		"roles":                 roleregistry.NewREST(roleregistry.NewVirtualRegistry(authorizationEtcd)),
 		"roleBindings":          rolebindingregistry.NewREST(rolebindingregistry.NewVirtualRegistry(authorizationEtcd, authorizationEtcd, c.Options.PolicyConfig.MasterAuthorizationNamespace)),
 		"resourceAccessReviews": resourceaccessreviewregistry.NewREST(c.Authorizer),
-		"subjectAccessReviews":  subjectaccessreviewregistry.NewREST(c.Authorizer),
+		"subjectAccessReviews":  subjectAccessReviewStorage,
 	}
 
 	admissionControl := admit.NewAlwaysAdmit()

pkg/image/registry/imagerepository/etcd/etcd.go

@@ -1,24 +1,33 @@
 package etcd
 
 import (
+	"fmt"
+	"strings"
+
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	etcdgeneric "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic/etcd"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
+	"github.com/golang/glog"
+	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
+	"github.com/openshift/origin/pkg/authorization/registry/subjectaccessreview"
 	"github.com/openshift/origin/pkg/image/api"
 	"github.com/openshift/origin/pkg/image/registry/imagerepository"
 )
 
 // REST implements a RESTStorage for image repositories against etcd.
 type REST struct {
-	store *etcdgeneric.Etcd
+	store                       *etcdgeneric.Etcd
+	subjectAccessReviewRegistry subjectaccessreview.Registry
+	defaultRegistry             imagerepository.DefaultRegistry
 }
 
 // NewREST returns a new REST.
-func NewREST(h tools.EtcdHelper, defaultRegistry imagerepository.DefaultRegistry) (*REST, *StatusREST) {
+func NewREST(h tools.EtcdHelper, defaultRegistry imagerepository.DefaultRegistry, subjectAccessReviewRegistry subjectaccessreview.Registry) (*REST, *StatusREST) {
 	prefix := "/imageRepositories"
 	store := etcdgeneric.Etcd{
 		NewFunc:     func() runtime.Object { return &api.ImageRepository{} },
@@ -47,7 +56,7 @@ func NewREST(h tools.EtcdHelper, defaultRegistry imagerepository.DefaultRegistry
 	store.UpdateStrategy = strategy
 	store.Decorator = strategy.Decorate
 
-	return &REST{store: &store}, &StatusREST{store: &statusStore}
+	return &REST{store: &store, subjectAccessReviewRegistry: subjectAccessReviewRegistry, defaultRegistry: defaultRegistry}, &StatusREST{store: &statusStore}
 }
 
 // New returns a new object
@@ -77,11 +86,73 @@ func (r *REST) Get(ctx kapi.Context, name string) (runtime.Object, error) {
 
 // Create creates a image repository based on a specification.
 func (r *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, error) {
+	repo := obj.(*api.ImageRepository)
+
+	user, ok := kapi.UserFrom(ctx)
+	if !ok {
+		return nil, kerrors.NewForbidden("imageRepository", repo.Name, fmt.Errorf("Unable to create an image repository without a user on the context"))
+	}
+	if errors := r.validateTagAccess(repo, user.GetName()); len(errors) > 0 {
+		return nil, kerrors.NewInvalid("imageRepository", repo.Name, errors)
+	}
+
 	return r.store.Create(ctx, obj)
 }
 
+func (r *REST) validateTagAccess(repo *api.ImageRepository, user string) kerrors.ValidationErrorList {
+	var errors kerrors.ValidationErrorList
+	if repo.Tags != nil {
+		for tag, value := range repo.Tags {
+			if !strings.Contains(value, "/") {
+				glog.V(1).Infof("Tag %q is local; skipping access check", value)
+				continue
+			}
+			ref, err := api.ParseDockerImageReference(value)
+			if err != nil {
+				errors = append(errors, kerrors.NewFieldInvalid("tags", tag, fmt.Sprintf("Unable to parse %q as a Docker image reference", value)))
+			}
+			glog.Infof("validating access for %s to %s", user, ref.String())
+			defaultRegistry, _ := r.defaultRegistry.DefaultRegistry()
+			if ref.Registry != "" && ref.Registry != defaultRegistry {
+				glog.Errorf("ref.Registry=%s, defaultRegistry=%s", ref.Registry, defaultRegistry)
+				//TODO should we see if we can find an IR whose spec.DockerImageRepository matches
+				//ref.Registry and do an access check against that?
+				glog.V(1).Infof("Tag %q points to external registry; skipping access check", value)
+				continue
+			}
+			if ref.Namespace == repo.Namespace {
+				glog.V(1).Infof("Tag %q points to a repo in the current namespace; skipping access check", value)
+				continue
+			}
+			subjectAccessReview := authorizationapi.SubjectAccessReview{
+				Verb:         "get",
+				Resource:     "imageRepository",
+				User:         user,
+				ResourceName: ref.Name,
+			}
+			ctx := kapi.WithNamespace(kapi.NewContext(), ref.Namespace)
+			glog.V(1).Infof("Performing SubjectAccessReview for user %s to %s/%s", user, ref.Namespace, ref.Name)
+			resp, err := r.subjectAccessReviewRegistry.CreateSubjectAccessReview(ctx, &subjectAccessReview)
+			if err != nil || !resp.Allowed {
+				errors = append(errors, kerrors.NewFieldForbidden("tags", fmt.Sprintf("%s=%s", tag, value)))
+			}
+		}
+	}
+	return errors
+}
+
 // Update changes a image repository specification.
 func (r *REST) Update(ctx kapi.Context, obj runtime.Object) (runtime.Object, bool, error) {
+	repo := obj.(*api.ImageRepository)
+
+	user, ok := kapi.UserFrom(ctx)
+	if !ok {
+		return nil, false, kerrors.NewForbidden("imageRepository", repo.Name, fmt.Errorf("Unable to update an image repository without a user on the context"))
+	}
+	if errors := r.validateTagAccess(repo, user.GetName()); len(errors) > 0 {
+		return nil, false, kerrors.NewInvalid("imageRepository", repo.Name, errors)
+	}
+
 	return r.store.Update(ctx, obj)
 }