Browse code

try to match forbidden status to api version request

deads2k authored on 2015/04/22 00:17:47
Showing 4 changed files
... ...
@@ -12,6 +12,7 @@ import (
12 12
 
13 13
 type DefaultAuthorizationAttributes struct {
14 14
 	Verb              string
15
+	APIVersion        string
15 16
 	Resource          string
16 17
 	ResourceName      string
17 18
 	RequestAttributes interface{}
... ...
@@ -105,6 +106,10 @@ func splitPath(thePath string) []string {
105 105
 	return strings.Split(thePath, "/")
106 106
 }
107 107
 
108
+func (a DefaultAuthorizationAttributes) GetAPIVersion() string {
109
+	return a.APIVersion
110
+}
111
+
108 112
 func (a DefaultAuthorizationAttributes) GetResource() string {
109 113
 	return a.Resource
110 114
 }
... ...
@@ -43,6 +43,7 @@ func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request
43 43
 
44 44
 	return DefaultAuthorizationAttributes{
45 45
 		Verb:              requestInfo.Verb,
46
+		APIVersion:        requestInfo.APIVersion,
46 47
 		Resource:          resource,
47 48
 		ResourceName:      requestInfo.Name,
48 49
 		RequestAttributes: req,
... ...
@@ -18,6 +18,7 @@ type AuthorizationAttributeBuilder interface {
18 18
 
19 19
 type AuthorizationAttributes interface {
20 20
 	GetVerb() string
21
+	GetAPIVersion() string
21 22
 	// GetResource returns the resource type.  If IsNonResourceURL() is true, then GetResource() is "".
22 23
 	GetResource() string
23 24
 	GetResourceName() string
... ...
@@ -578,27 +578,27 @@ func (c *MasterConfig) authorizationFilter(handler http.Handler) http.Handler {
578 578
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
579 579
 		attributes, err := c.AuthorizationAttributeBuilder.GetAttributes(req)
580 580
 		if err != nil {
581
-			forbidden(err.Error(), w, req)
581
+			forbidden(err.Error(), "", w, req)
582 582
 			return
583 583
 		}
584 584
 		if attributes == nil {
585
-			forbidden("No attributes", w, req)
585
+			forbidden("No attributes", "", w, req)
586 586
 			return
587 587
 		}
588 588
 
589 589
 		ctx, exists := c.RequestContextMapper.Get(req)
590 590
 		if !exists {
591
-			forbidden("context not found", w, req)
591
+			forbidden("context not found", attributes.GetAPIVersion(), w, req)
592 592
 			return
593 593
 		}
594 594
 
595 595
 		allowed, reason, err := c.Authorizer.Authorize(ctx, attributes)
596 596
 		if err != nil {
597
-			forbidden(err.Error(), w, req)
597
+			forbidden(err.Error(), attributes.GetAPIVersion(), w, req)
598 598
 			return
599 599
 		}
600 600
 		if !allowed {
601
-			forbidden(reason, w, req)
601
+			forbidden(reason, attributes.GetAPIVersion(), w, req)
602 602
 			return
603 603
 		}
604 604
 
... ...
@@ -607,7 +607,15 @@ func (c *MasterConfig) authorizationFilter(handler http.Handler) http.Handler {
607 607
 }
608 608
 
609 609
 // forbidden renders a simple forbidden error
610
-func forbidden(reason string, w http.ResponseWriter, req *http.Request) {
610
+func forbidden(reason, apiVersion string, w http.ResponseWriter, req *http.Request) {
611
+	// the api version can be empty for two basic reasons:
612
+	// 1. malformed API request
613
+	// 2. not an API request at all
614
+	// In these cases, just assume the latest version that will work better than nothing
615
+	if len(apiVersion) == 0 {
616
+		apiVersion = klatest.Version
617
+	}
618
+
611 619
 	// Reason is an opaque string that describes why access is allowed or forbidden (forbidden by the time we reach here).
612 620
 	// We don't have direct access to kind or name (not that those apply either in the general case)
613 621
 	// We create a NewForbidden to stay close the API, but then we override the message to get a serialization
... ...
@@ -615,12 +623,15 @@ func forbidden(reason string, w http.ResponseWriter, req *http.Request) {
615 615
 	forbiddenError, _ := kapierror.NewForbidden("", "", errors.New("")).(*kapierror.StatusError)
616 616
 	forbiddenError.ErrStatus.Message = fmt.Sprintf("%q is forbidden because %s", req.RequestURI, reason)
617 617
 
618
-	// TODO: this serialization is broken in the general case.  It chooses the latest, but
619
-	// it should choose the serialization based on the codec for the uri requested.  What is coded here
620
-	// is better than not returning a status object at all, but needs to be fixed once authorization is
621
-	// tied into the APIInstaller itself.  Then we'll only have the wrong codec for non-resource requests
618
+	// Not all API versions in valid API requests will have a matching codec in kubernetes.  If we can't find one,
619
+	// just default to the latest kube codec.
620
+	codec := klatest.Codec
621
+	if requestedCodec, err := klatest.InterfacesFor(apiVersion); err == nil {
622
+		codec = requestedCodec
623
+	}
624
+
622 625
 	formatted := &bytes.Buffer{}
623
-	output, err := klatest.Codec.Encode(&forbiddenError.ErrStatus)
626
+	output, err := codec.Encode(&forbiddenError.ErrStatus)
624 627
 	if err != nil {
625 628
 		fmt.Fprintf(formatted, "%s", forbiddenError.Error())
626 629
 	} else {