[ARVADOS] updated: eef1936a73c7c85ba530cca028230241f5171333
git at public.curoverse.com
git at public.curoverse.com
Fri May 9 23:14:05 EDT 2014
Summary of changes:
services/keep/src/keep/handler_test.go | 138 ++++++++++++++++++++-------------
services/keep/src/keep/keep.go | 19 ++---
2 files changed, 93 insertions(+), 64 deletions(-)
via eef1936a73c7c85ba530cca028230241f5171333 (commit)
from 1a844d06238368c9d5c946a34c0c52485de1c435 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit eef1936a73c7c85ba530cca028230241f5171333
Author: Tim Pierce <twp at curoverse.com>
Date: Fri May 9 23:12:18 2014 -0400
2328: restrict all /index requests to superuser
Per discussion with Tom on IRC: all /index requests, whether they
include a prefix argument or not, should be restricted to the superuser.
diff --git a/services/keep/src/keep/handler_test.go b/services/keep/src/keep/handler_test.go
index 9455a8a..08256f7 100644
--- a/services/keep/src/keep/handler_test.go
+++ b/services/keep/src/keep/handler_test.go
@@ -190,105 +190,137 @@ func TestPutHandler(t *testing.T) {
}
// Test /index requests:
-// - unauthenticated /index/{prefix} request
-// - unauthenticated /index request
-// - authenticated /index request, non-superuser
-// - authenticated /index request by superuser, enforce_permissions = false
-// - authenticated /index request by superuser, enforce_permissions = true
+// - enforce_permissions off, unauthenticated /index request
+// - enforce_permissions off, authenticated /index request, non-superuser
+// - enforce_permissions off, authenticated /index request, superuser
+// - enforce_permissions on, unauthenticated /index request
+// - enforce_permissions on, authenticated /index request, non-superuser
+// - enforce_permissions on, authenticated /index request, superuser
+// - enforce_permissions on, authenticated /index/prefix request, superuser
+//
+// The only /index requests that should succeed are those issued by the
+// superuser when enforce_permissions = true.
//
func TestIndexHandler(t *testing.T) {
defer teardown()
// Set up Keep volumes and populate them.
// Include multiple blocks on different volumes, and
- // some metadata files.
+ // some metadata files (which should be omitted from index listings)
KeepVM = MakeTestVolumeManager(2)
defer func() { KeepVM.Quit() }()
vols := KeepVM.Volumes()
vols[0].Put(TEST_HASH, TEST_BLOCK)
vols[1].Put(TEST_HASH_2, TEST_BLOCK_2)
+ vols[0].Put(TEST_HASH+".meta", []byte("metadata"))
+ vols[1].Put(TEST_HASH_2+".meta", []byte("metadata"))
// Set up a REST router for testing the handlers.
rest := NewRESTRouter()
- // Unauthenticated /index/{prefix}
- // => OK
- response := IssueRequest(rest,
- &RequestTester{
- method: "GET",
- uri: "http://localhost:25107/index/" + TEST_HASH[0:5],
- })
+ data_manager_token = "DATA MANAGER TOKEN"
- expected := `^` + TEST_HASH + `\+\d+ \d+\n$`
- match, _ := regexp.MatchString(expected, response.Body.String())
- if !match {
- t.Errorf("IndexHandler expected: %s, returned:\n%s",
- expected, response.Body.String())
+ unauthenticated_req := &RequestTester{
+ method: "GET",
+ uri: "http://localhost:25107/index",
+ }
+ authenticated_req := &RequestTester{
+ method: "GET",
+ uri: "http://localhost:25107/index",
+ api_token: known_token,
+ }
+ superuser_req := &RequestTester{
+ method: "GET",
+ uri: "http://localhost:25107/index",
+ api_token: data_manager_token,
}
- // Unauthenticated /index
- // => PermissionError
- response = IssueRequest(rest,
- &RequestTester{
- method: "GET",
- uri: "http://localhost:25107/index",
- })
+ // ----------------------------
+ // enforce_permissions disabled
+ // All /index requests should fail.
+ enforce_permissions = false
+ // unauthenticated /index request
+ // => PermissionError
+ response := IssueRequest(rest, unauthenticated_req)
ExpectStatusCode(t,
- "unauthenticated /index", PermissionError.HTTPCode, response)
+ "enforce_permissions off, unauthenticated request",
+ PermissionError.HTTPCode,
+ response)
- // Authenticated /index request by non-superuser
+ // authenticated /index request, non-superuser
// => PermissionError
- response = IssueRequest(rest,
- &RequestTester{
- method: "GET",
- uri: "http://localhost:25107/index",
- api_token: known_token,
- })
-
+ response = IssueRequest(rest, authenticated_req)
ExpectStatusCode(t,
- "authenticated /index by non-superuser",
+ "enforce_permissions off, authenticated request, non-superuser",
PermissionError.HTTPCode,
response)
- // Authenticated /index request by superuser, enforce_permissions = false
+ // authenticated /index request, superuser
// => PermissionError
- enforce_permissions = false
- data_manager_token = "DATA MANAGER TOKEN"
+ response = IssueRequest(rest, superuser_req)
+ ExpectStatusCode(t,
+ "enforce_permissions off, superuser request",
+ PermissionError.HTTPCode,
+ response)
- response = IssueRequest(rest,
- &RequestTester{
- method: "GET",
- uri: "http://localhost:25107/index",
- api_token: data_manager_token,
- })
+ // ---------------------------
+ // enforce_permissions enabled
+ // Only the superuser should be allowed to issue /index requests.
+ enforce_permissions = true
+
+ // unauthenticated /index request
+ // => PermissionError
+ response = IssueRequest(rest, unauthenticated_req)
+ ExpectStatusCode(t,
+ "enforce_permissions on, unauthenticated request",
+ PermissionError.HTTPCode,
+ response)
+ // authenticated /index request, non-superuser
+ // => PermissionError
+ response = IssueRequest(rest, authenticated_req)
ExpectStatusCode(t,
- "authenticated /index request by superuser (permissions off)",
+ "permissions on, authenticated request, non-superuser",
PermissionError.HTTPCode,
response)
- // Authenticated /index request by superuser, enforce_permissions = true
+ // superuser /index request
+ // => OK
+ response = IssueRequest(rest, superuser_req)
+ ExpectStatusCode(t,
+ "permissions on, superuser request",
+ http.StatusOK,
+ response)
+
+ expected := `^` + TEST_HASH + `\+\d+ \d+\n` +
+ TEST_HASH_2 + `\+\d+ \d+\n$`
+ match, _ := regexp.MatchString(expected, response.Body.String())
+ if !match {
+ t.Errorf(
+ "permissions on, superuser request: expected %s, got:\n%s",
+ expected, response.Body.String())
+ }
+
+ // superuser /index/prefix request
// => OK
- enforce_permissions = true
response = IssueRequest(rest,
&RequestTester{
method: "GET",
- uri: "http://localhost:25107/index",
+ uri: "http://localhost:25107/index/" + TEST_HASH[0:3],
api_token: data_manager_token,
})
-
ExpectStatusCode(t,
- "authenticated /index request by superuser (permissions on)",
+ "permissions on, superuser request",
http.StatusOK,
response)
- expected = `^` + TEST_HASH + `\+\d+ \d+\n` +
- TEST_HASH_2 + `\+\d+ \d+\n$`
+ expected = `^` + TEST_HASH + `\+\d+ \d+\n$`
match, _ = regexp.MatchString(expected, response.Body.String())
if !match {
- t.Errorf("superuser /index: expected %s, got:\n%s",
+ t.Errorf(
+ "permissions on, superuser /index/prefix request: expected %s, got:\n%s",
expected, response.Body.String())
}
}
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index 143d290..0fdf660 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -335,18 +335,15 @@ func PutBlockHandler(w http.ResponseWriter, req *http.Request) {
func IndexHandler(w http.ResponseWriter, req *http.Request) {
prefix := mux.Vars(req)["prefix"]
- // Only the data manager may issue unqualified "GET /index" requests,
+ // Only the data manager may issue /index requests,
// and only if enforce_permissions is enabled.
- // If the request is unauthenticated, or does not match the data manager's
- // API token, return 403 Permission denied.
- if prefix == "" {
- api_token := GetApiToken(req)
- if !enforce_permissions ||
- api_token == "" ||
- data_manager_token != GetApiToken(req) {
- http.Error(w, PermissionError.Error(), PermissionError.HTTPCode)
- return
- }
+ // All other requests return 403 Permission denied.
+ api_token := GetApiToken(req)
+ if !enforce_permissions ||
+ api_token == "" ||
+ data_manager_token != GetApiToken(req) {
+ http.Error(w, PermissionError.Error(), PermissionError.HTTPCode)
+ return
}
var index string
for _, vol := range KeepVM.Volumes() {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list