Use "nonResourcePath" property in policy to expose custom endpoints.

Fixed indents and replaced 'kind' w/ 'resource'.

Fixed some more indents.

Tests: Replaced kind w/ resource once again, some minor tweaks.

Fixed indents.

Fixed indents.

Only use the API request resolver on actual api prefixed requests.

Updated doc for swagger policies.

Address gofmt complaints.

Consider api prefixes with more than 1 path fragment. ("api/123")

Fix: incrementor missing.

Refactored and tests added.

Allow trailing slash.

Addressed kubernetes#3, kubernetes#4, kubernetes#6, kubernetes#5, kubernetes#8, kubernetes#7

Enforce non-resource auth on all users, not only ns'ed.

Conflicts:
	docs/admin/authorization.md
	pkg/apiserver/handlers_test.go
	pkg/auth/authorizer/abac/abac_test.go

docs/admin/authorization.md

@@ -29,15 +29,22 @@ The following implementations are available, and are selected by flag:
 
 ### Request Attributes
 
+<<<<<<< HEAD
 A request has 4 attributes that can be considered for authorization:
+=======
+A request has 6 attributes that can be considered for authorization:
+>>>>>>> a55592e... Use "nonResourcePath" property in policy to expose custom endpoints.
   - user (the user-string which a user was authenticated as).
-  - whether the request is readonly (GETs are readonly)
-  - what resource is being accessed 
-    - applies only to the API endpoints, such as 
+  - group (the list of group names the authenticated user is a member of).
+  - whether the request is readonly (GETs are readonly).
+  - what resource is being accessed.
+    - applies only to the API endpoints, such as
         `/api/v1/namespaces/default/pods`.  For miscellaneous endpoints, like `/version`, the
-        resource is the empty string.
+        resource is the empty string, and the non resource path is populated instead.
   - the namespace of the object being access, or the empty string if the
         endpoint does not support namespaced objects.
+  - a non resource path for providing access to miscellaneous endpoints like `/api` or `/healthz` (see [kubectl](#kubectl)).
+    - This cannot be used to provide access to namespaces/resources via their full path, only non-resource paths will be considered.
 
 We anticipate adding more attributes to allow finer grained access control and
 to assist in policy management.
@@ -55,6 +62,7 @@ Each line is a "policy object".  A policy object is a map with the following pro
       operations.
   - `resource`, type string; a resource from an URL, such as `pods`.
   - `namespace`, type string; a namespace string.
+  - `nonResourcePath`, type string; when it's set everything but the `readonly` property is ignored.
 
 An unset property is the same as a property set to the zero value for its type (e.g. empty string, 0, false).
 However, unset should be preferred for readability.
@@ -78,12 +86,27 @@ If at least one line matches the request attributes, then the request is authori
 To permit any user to do something, write a policy with the user property unset.
 To permit an action Policy with an unset namespace applies regardless of namespace.
 
+### Kubectl
+
+Kubectl uses the `/api` endpoint of api-server to negotiate client/server versions. To validate objects send to the API by create/update operations, kubectl queries certain swagger resources. For API version `v1` those would be `/swaggerapi/api/v1` & `/swaggerapi/experimental/v1`.
+
+When using ABAC authorization, those special resources have to be explicitly exposed via the `nonResourcePath` property in a policy (see [examples](#examples) below):
+
+* `/api` for api version negotiation.
+* `/version` for retrieving the api-server version via `kubectl version`.
+* `/swaggerapi/api/v1"` & `/swaggerapi/experimental/v1` for create/update operations.
+
+To inspect the HTTP calls involved in a specific kubectl operation you can turn up the verbosity:
+
+    kubectl --v=10 version
+
 ### Examples
 
- 1. Alice can do anything: `{"user":"alice"}`
+ 1. Alice can do anything to all resources in all namespaces: `{"user":"alice"}`
  2. Kubelet can read any pods: `{"user":"kubelet", "resource": "pods", "readonly": true}`
  3. Kubelet can read and write events: `{"user":"kubelet", "resource": "events"}`
- 4. Bob can just read pods in namespace "projectCaribou": `{"user":"bob", "resource": "pods", "readonly": true, "ns": "projectCaribou"}`
+ 4. Bob can just read pods in namespace "projectCaribou": `{"user":"bob", "resource": "pods", "readonly": true, "namespace": "projectCaribou"}`
+ 5. Anyone can retrieve a list of available api versions: `{"readonly": "true", "nonResourcePath": "/api"}`
 
 [Complete file example](http://releases.k8s.io/v1.0.6/pkg/auth/authorizer/abac/example_policy_file.jsonl)
 

pkg/apiserver/handlers.go

@@ -192,6 +192,19 @@ func NewRequestAttributeGetter(requestContextMapper api.RequestContextMapper, re
 	return &requestAttributeGetter{requestContextMapper, &APIRequestInfoResolver{util.NewStringSet(apiRoots...), restMapper}}
 }
 
+func isAPIResourceRequest(apiPrefixes util.StringSet, req *http.Request) bool {
+	// Slice the first / of the path off (in apiRoots they're stored w/o leading /)
+	parts := strings.Split(req.URL.Path, "/")[1:]
+	// Paths with only one fragment won't have a prefix.
+	for i := 1; i < len(parts); i++ {
+		prefix := strings.Join(parts[:i], "/")
+		if apiPrefixes.Has(prefix) {
+			return true
+		}
+	}
+	return false
+}
+
 func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attributes {
 	attribs := authorizer.AttributesRecord{}
 
@@ -205,16 +218,22 @@ func (r *requestAttributeGetter) GetAttribs(req *http.Request) authorizer.Attrib
 
 	attribs.ReadOnly = IsReadOnlyReq(*req)
 
-	apiRequestInfo, _ := r.apiRequestInfoResolver.GetAPIRequestInfo(req)
+	// Check whether meaningful api information can be resolved for the current path
+	if isAPIResourceRequest(r.apiRequestInfoResolver.APIPrefixes, req) {
+		apiRequestInfo, _ := r.apiRequestInfoResolver.GetAPIRequestInfo(req)
 
-	// If a path follows the conventions of the REST object store, then
-	// we can extract the resource.  Otherwise, not.
-	attribs.Resource = apiRequestInfo.Resource
+		// If a path follows the conventions of the REST object store, then
+		// we can extract the resource.  Otherwise, not.
+		attribs.Resource = apiRequestInfo.Resource
 
-	// If the request specifies a namespace, then the namespace is filled in.
-	// Assumes there is no empty string namespace.  Unspecified results
-	// in empty (does not understand defaulting rules.)
-	attribs.Namespace = apiRequestInfo.Namespace
+		// If the request specifies a namespace, then the namespace is filled in.
+		// Assumes there is no empty string namespace.  Unspecified results
+		// in empty (does not understand defaulting rules.)
+		attribs.Namespace = apiRequestInfo.Namespace
+	} else {
+		// If a request does not fall into an api namespace/resource pattern, it's a special path.
+		attribs.NonResourcePath = req.URL.Path
+	}
 
 	return &attribs
 }

pkg/apiserver/handlers_test.go

@@ -143,6 +143,129 @@ func TestReadOnly(t *testing.T) {
 	}
 }
 
+func TestTimeout(t *testing.T) {
+	sendResponse := make(chan struct{}, 1)
+	writeErrors := make(chan error, 1)
+	timeout := make(chan time.Time, 1)
+	resp := "test response"
+	timeoutResp := "test timeout"
+
+	ts := httptest.NewServer(TimeoutHandler(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			<-sendResponse
+			_, err := w.Write([]byte(resp))
+			writeErrors <- err
+		}),
+		func(*http.Request) (<-chan time.Time, string) {
+			return timeout, timeoutResp
+		}))
+	defer ts.Close()
+
+	// No timeouts
+	sendResponse <- struct{}{}
+	res, err := http.Get(ts.URL)
+	if err != nil {
+		t.Error(err)
+	}
+	if res.StatusCode != http.StatusOK {
+		t.Errorf("got res.StatusCode %d; expected %d", res.StatusCode, http.StatusOK)
+	}
+	body, _ := ioutil.ReadAll(res.Body)
+	if string(body) != resp {
+		t.Errorf("got body %q; expected %q", string(body), resp)
+	}
+	if err := <-writeErrors; err != nil {
+		t.Errorf("got unexpected Write error on first request: %v", err)
+	}
+
+	// Times out
+	timeout <- time.Time{}
+	res, err = http.Get(ts.URL)
+	if err != nil {
+		t.Error(err)
+	}
+	if res.StatusCode != http.StatusGatewayTimeout {
+		t.Errorf("got res.StatusCode %d; expected %d", res.StatusCode, http.StatusServiceUnavailable)
+	}
+	body, _ = ioutil.ReadAll(res.Body)
+	if string(body) != timeoutResp {
+		t.Errorf("got body %q; expected %q", string(body), timeoutResp)
+	}
+
+	// Now try to send a response
+	sendResponse <- struct{}{}
+	if err := <-writeErrors; err != http.ErrHandlerTimeout {
+		t.Errorf("got Write error of %v; expected %v", err, http.ErrHandlerTimeout)
+	}
+}
+
+func TestIsAPIResourceRequest(t *testing.T) {
+	testCases := []struct {
+		apiRoots       []string
+		path           string
+		expectedResult bool
+	}{
+		{[]string{"api"}, "/abc", false},
+		{[]string{"api"}, "/api/abc", true},
+		// A prefixed path requires at least two fragments
+		{[]string{"api"}, "/api", false},
+		{[]string{"api"}, "/api/", true},
+		// It might be more than one prefix available
+		{[]string{"api", "test"}, "/test/123/abc", true},
+		// The prefix might be two or more fragments
+		{[]string{"test/123"}, "/test/123/abc", true},
+		{[]string{"test/123"}, "/test/123/", true},
+		{[]string{"test/123"}, "/test/123", false},
+	}
+
+	for i, tc := range testCases {
+		t.Logf("tc %v: %v", i, tc)
+		req, _ := http.NewRequest("GET", tc.path, nil)
+		has := isAPIResourceRequest(util.NewStringSet(tc.apiRoots...), req)
+		if has != tc.expectedResult {
+			t.Errorf("expected %v, was %v", tc.expectedResult, has)
+		}
+	}
+}
+
+func TestGetAttribs(t *testing.T) {
+	r := &requestAttributeGetter{api.NewRequestContextMapper(), &APIRequestInfoResolver{util.NewStringSet("api"), nil}}
+
+	// When path does not start with an api prefix, it's a non-resource path.
+	path := "/version"
+	req, _ := http.NewRequest("GET", path, nil)
+	attribs := r.GetAttribs(req)
+	nonResourcePath := attribs.GetNonResourcePath()
+	if nonResourcePath != path {
+		t.Errorf("Expected %s, is %+v", path, nonResourcePath)
+	}
+	namespace := attribs.GetNamespace()
+	if namespace != "" {
+		t.Errorf("Expected empty, is %+v", namespace)
+	}
+	resource := attribs.GetResource()
+	if resource != "" {
+		t.Errorf("Expected empty, is %+v", resource)
+	}
+
+	// Neither non-resource nor resource path when it starts with an api prefix.
+	path = "/api/unknown"
+	req, _ = http.NewRequest("GET", path, nil)
+	attribs = r.GetAttribs(req)
+	nonResourcePath = attribs.GetNonResourcePath()
+	if nonResourcePath != "" {
+		t.Errorf("Expected empty, is %+v", nonResourcePath)
+	}
+	namespace = attribs.GetNamespace()
+	if namespace != "" {
+		t.Errorf("Expected empty, is %+v", namespace)
+	}
+	resource = attribs.GetResource()
+	if resource != "" {
+		t.Errorf("Expected empty, is %+v", resource)
+	}
+}
+
 func TestGetAPIRequestInfo(t *testing.T) {
 	successCases := []struct {
 		method              string

pkg/auth/authorizer/abac/abac.go

@@ -51,9 +51,10 @@ type policy struct {
 	// the API, we don't have to add lots of policy?
 
 	// TODO: make this a proper REST object with its own registry.
-	Readonly  bool   `json:"readonly,omitempty"`
-	Resource  string `json:"resource,omitempty"`
-	Namespace string `json:"namespace,omitempty"`
+	Readonly        bool   `json:"readonly,omitempty"`
+	Resource        string `json:"resource,omitempty"`
+	Namespace       string `json:"namespace,omitempty"`
+	NonResourcePath string `json:"nonResourcePath,omitempty"`
 
 	// TODO: "expires" string in RFC3339 format.
 
@@ -100,13 +101,20 @@ func NewFromFile(path string) (policyList, error) {
 func (p policy) matches(a authorizer.Attributes) bool {
 	if p.subjectMatches(a) {
 		if p.Readonly == false || (p.Readonly == a.IsReadOnly()) {
-			if p.Resource == "" || (p.Resource == a.GetResource()) {
+			switch {
+			case p.NonResourcePath != "":
+				if p.NonResourcePath == a.GetNonResourcePath() {
+					return true
+				}
+			// When the path is a non-resource path it cannot match.
+			case len(a.GetNonResourcePath()) == 0 && (p.Resource == "" || (p.Resource == a.GetResource())):
 				if p.Namespace == "" || (p.Namespace == a.GetNamespace()) {
 					return true
 				}
 			}
 		}
 	}
+
 	return false
 }
 

pkg/auth/authorizer/abac/abac_test.go

@@ -56,29 +56,35 @@ func TestExampleFile(t *testing.T) {
 	}
 }
 
-func NotTestAuthorize(t *testing.T) {
-	a, err := newWithContents(t, `{                     "readonly": true, "kind": "events"}
-{"user":"scheduler",  "readonly": true, "kind": "pods"}
-{"user":"scheduler",              "kind": "bindings"}
-{"user":"kubelet",    "readonly": true, "kind": "bindings"}
-{"user":"kubelet",                "kind": "events"}
-{"user":"alice",                                     "ns": "projectCaribou"}
-{"user":"bob",        "readonly": true,                    "ns": "projectCaribou"}
-`)
+func TestAuthorize(t *testing.T) {
+	a, err := newWithContents(t,
+		`{                    "readonly": true,                                                        "nonResourcePath": "/api"}
+		 {                                                                                             "nonResourcePath": "/custom"}
+		 {                    "readonly": true, "resource": "events"}
+		 {"user":"scheduler", "readonly": true, "resource": "pods"}
+		 {"user":"scheduler",                   "resource": "bindings"}
+		 {"user":"kubelet",   "readonly": true, "resource": "bindings"}
+		 {"user":"kubelet",                     "resource": "events"}
+		 {"user":"alice",                                               "namespace": "projectCaribou"}
+		 {"user":"bob",       "readonly": true,                         "namespace": "projectCaribou"}
+		 {"user":"debbie",                      "resource": "pods",     "namespace": "projectCaribou"}`)
+
 	if err != nil {
 		t.Fatalf("unable to read policy file: %v", err)
 	}
 
 	uScheduler := user.DefaultInfo{Name: "scheduler", UID: "uid1"}
 	uAlice := user.DefaultInfo{Name: "alice", UID: "uid3"}
 	uChuck := user.DefaultInfo{Name: "chuck", UID: "uid5"}
+	uDebbie := user.DefaultInfo{Name: "debbie", UID: "uid6"}
 
 	testCases := []struct {
-		User        user.DefaultInfo
-		RO          bool
-		Resource    string
-		NS          string
-		ExpectAllow bool
+		User            user.DefaultInfo
+		RO              bool
+		Resource        string
+		NS              string
+		NonResourcePath string
+		ExpectAllow     bool
 	}{
 		// Scheduler can read pods
 		{User: uScheduler, RO: true, Resource: "pods", NS: "ns1", ExpectAllow: true},
@@ -102,31 +108,44 @@ func NotTestAuthorize(t *testing.T) {
 		{User: uAlice, RO: true, Resource: "widgets", NS: "ns1", ExpectAllow: false},
 		{User: uAlice, RO: true, Resource: "", NS: "ns1", ExpectAllow: false},
 
+		// Debbie can write to pods in the right namespace
+		{User: uDebbie, RO: false, Resource: "pods", NS: "projectCaribou", ExpectAllow: true},
+
 		// Chuck can read events, since anyone can.
 		{User: uChuck, RO: true, Resource: "events", NS: "ns1", ExpectAllow: true},
 		{User: uChuck, RO: true, Resource: "events", NS: "", ExpectAllow: true},
 		// Chuck can't do other things.
 		{User: uChuck, RO: false, Resource: "events", NS: "ns1", ExpectAllow: false},
 		{User: uChuck, RO: true, Resource: "pods", NS: "ns1", ExpectAllow: false},
 		{User: uChuck, RO: true, Resource: "floop", NS: "ns1", ExpectAllow: false},
-		// Chunk can't access things with no kind or namespace
-		// TODO: find a way to give someone access to miscelaneous endpoints, such as
-		// /healthz, /version, etc.
+		// Chuck can't access things with no resource or namespace
 		{User: uChuck, RO: true, Resource: "", NS: "", ExpectAllow: false},
+		// but can access /api
+		{User: uChuck, RO: true, NonResourcePath: "/api", Resource: "", NS: "", ExpectAllow: true},
+		// though he cannot write to it
+		{User: uChuck, RO: false, NonResourcePath: "/api", Resource: "", NS: "", ExpectAllow: false},
+		// while he can write to /custom
+		{User: uChuck, RO: false, NonResourcePath: "/custom", Resource: "", NS: "", ExpectAllow: true},
+
+		// When the path is a non-resource path it must not match resources/ns.
+		{User: uAlice, RO: false, NonResourcePath: "/something", Resource: "", NS: "projectCaribou", ExpectAllow: false},
+		{User: uScheduler, RO: false, NonResourcePath: "/something", Resource: "bindings", NS: "", ExpectAllow: false},
+		{User: uDebbie, RO: false, NonResourcePath: "", Resource: "pods", NS: "projectCaribou", ExpectAllow: true},
 	}
 	for _, tc := range testCases {
 		attr := authorizer.AttributesRecord{
-			User:      &tc.User,
-			ReadOnly:  tc.RO,
-			Resource:  tc.Resource,
-			Namespace: tc.NS,
+			User:            &tc.User,
+			ReadOnly:        tc.RO,
+			Resource:        tc.Resource,
+			Namespace:       tc.NS,
+			NonResourcePath: tc.NonResourcePath,
 		}
-		t.Logf("tc: %v -> attr %v", tc, attr)
+		t.Logf("tc %2v: %v -> attr %v", i, tc, attr)
 		err := a.Authorize(attr)
 		actualAllow := bool(err == nil)
 		if tc.ExpectAllow != actualAllow {
-			t.Errorf("Expected allowed=%v but actually allowed=%v, for case %v",
-				tc.ExpectAllow, actualAllow, tc)
+			t.Errorf("Expected allowed=%v but actually allowed=%v, for case %+v & %+v",
+				tc.ExpectAllow, actualAllow, tc, attr)
 		}
 	}
 }