[ARVADOS] updated: 3e6368d077fa4831f93255e271a18842788183fb
git at public.curoverse.com
git at public.curoverse.com
Tue May 13 14:10:31 EDT 2014
Summary of changes:
services/keep/src/keep/handler_test.go | 147 ++++++++++++++++++++++++---------
services/keep/src/keep/keep.go | 36 +++++---
2 files changed, 134 insertions(+), 49 deletions(-)
via 3e6368d077fa4831f93255e271a18842788183fb (commit)
from eef1936a73c7c85ba530cca028230241f5171333 (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 3e6368d077fa4831f93255e271a18842788183fb
Author: Tim Pierce <twp at curoverse.com>
Date: Tue May 13 14:05:45 2014 -0400
2328: incorporate code review comments.
Wrap very long lines to 80 columns.
Describe each test failure more explicitly.
TestIndexHandler: Add /index/prefix tests for unauthenticated and
authenticated non-superusers.
TestGetHandler: initialize all variables in one var block.
main: failure to read permission key or data manager token is now a
fatal error.
diff --git a/services/keep/src/keep/handler_test.go b/services/keep/src/keep/handler_test.go
index 08256f7..a2929c5 100644
--- a/services/keep/src/keep/handler_test.go
+++ b/services/keep/src/keep/handler_test.go
@@ -50,34 +50,41 @@ func TestGetHandler(t *testing.T) {
// Set up a REST router for testing the handlers.
rest := NewRESTRouter()
+ // Create locators for testing.
+ // Turn on permission settings so we can generate signed locators.
+ enforce_permissions = true
+ PermissionSecret = []byte(known_key)
+ permission_ttl = time.Duration(300) * time.Second
+
+ var (
+ unsigned_locator = "http://localhost:25107/" + TEST_HASH
+ valid_timestamp = time.Now().Add(permission_ttl)
+ expired_timestamp = time.Now().Add(-time.Hour)
+ signed_locator = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, valid_timestamp)
+ expired_locator = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, expired_timestamp)
+ )
+
// -----------------
- // Permissions: off.
+ // Test unauthenticated request with permissions off.
+ enforce_permissions = false
// Unauthenticated request, unsigned locator
// => OK
- unsigned_locator := "http://localhost:25107/" + TEST_HASH
response := IssueRequest(rest,
&RequestTester{
method: "GET",
uri: unsigned_locator,
})
- ExpectStatusCode(t, "unsigned GET (permissions off)", http.StatusOK, response)
- ExpectBody(t, "unsigned GET (permissions off)", string(TEST_BLOCK), response)
+ ExpectStatusCode(t,
+ "Unauthenticated request, unsigned locator", http.StatusOK, response)
+ ExpectBody(t,
+ "Unauthenticated request, unsigned locator",
+ string(TEST_BLOCK),
+ response)
// ----------------
// Permissions: on.
-
- // Create signed and expired locators for testing.
enforce_permissions = true
- PermissionSecret = []byte(known_key)
- permission_ttl = time.Duration(300) * time.Second
-
- var (
- expiration = time.Now().Add(permission_ttl)
- expired_timestamp = time.Now().Add(-time.Hour)
- signed_locator = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, expiration)
- expired_locator = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, expired_timestamp)
- )
// Authenticated request, signed locator
// => OK
@@ -86,8 +93,10 @@ func TestGetHandler(t *testing.T) {
uri: signed_locator,
api_token: known_token,
})
- ExpectStatusCode(t, "signed GET (permissions on)", http.StatusOK, response)
- ExpectBody(t, "signed GET (permissions on)", string(TEST_BLOCK), response)
+ ExpectStatusCode(t,
+ "Authenticated request, signed locator", http.StatusOK, response)
+ ExpectBody(t,
+ "Authenticated request, signed locator", string(TEST_BLOCK), response)
// Authenticated request, unsigned locator
// => PermissionError
@@ -104,7 +113,9 @@ func TestGetHandler(t *testing.T) {
method: "GET",
uri: signed_locator,
})
- ExpectStatusCode(t, "signed locator", PermissionError.HTTPCode, response)
+ ExpectStatusCode(t,
+ "Unauthenticated request, signed locator",
+ PermissionError.HTTPCode, response)
// Authenticated request, expired locator
// => ExpiredError
@@ -113,7 +124,9 @@ func TestGetHandler(t *testing.T) {
uri: expired_locator,
api_token: known_token,
})
- ExpectStatusCode(t, "expired signature", ExpiredError.HTTPCode, response)
+ ExpectStatusCode(t,
+ "Authenticated request, expired locator",
+ ExpiredError.HTTPCode, response)
}
// Test PutBlockHandler on the following situations:
@@ -145,8 +158,8 @@ func TestPutHandler(t *testing.T) {
})
ExpectStatusCode(t,
- "unauthenticated PUT (no server key)", http.StatusOK, response)
- ExpectBody(t, "unauthenticated PUT (no server key)", TEST_HASH, response)
+ "Unauthenticated request, no server key", http.StatusOK, response)
+ ExpectBody(t, "Unauthenticated request, no server key", TEST_HASH, response)
// ------------------
// With a server key.
@@ -168,9 +181,11 @@ func TestPutHandler(t *testing.T) {
})
ExpectStatusCode(t,
- "authenticated PUT (with server key)", http.StatusOK, response)
+ "Authenticated PUT, signed locator, with server key",
+ http.StatusOK, response)
if !VerifySignature(response.Body.String(), known_token) {
- t.Errorf("authenticated PUT (with server key): response '%s' does not contain a valid signature",
+ t.Errorf("Authenticated PUT, signed locator, with server key:\n"+
+ "response '%s' does not contain a valid signature",
response.Body.String())
}
@@ -184,19 +199,26 @@ func TestPutHandler(t *testing.T) {
})
ExpectStatusCode(t,
- "unauthenticated PUT (with server key)", http.StatusOK, response)
+ "Unauthenticated PUT, unsigned locator, with server key",
+ http.StatusOK, response)
ExpectBody(t,
- "unauthenticated PUT (with server key)", TEST_HASH, response)
+ "Unauthenticated PUT, unsigned locator, with server key",
+ TEST_HASH, response)
}
// Test /index requests:
-// - 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
+// - enforce_permissions off | unauthenticated /index request
+// - enforce_permissions off | unauthenticated /index/prefix request
+// - enforce_permissions off | authenticated /index request | non-superuser
+// - enforce_permissions off | authenticated /index/prefix request | non-superuser
+// - enforce_permissions off | authenticated /index request | superuser
+// - enforce_permissions off | authenticated /index/prefix request | superuser
+// - enforce_permissions on | unauthenticated /index request
+// - enforce_permissions on | unauthenticated /index/prefix request
+// - enforce_permissions on | authenticated /index request | non-superuser
+// - enforce_permissions on | authenticated /index/prefix 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.
@@ -235,6 +257,20 @@ func TestIndexHandler(t *testing.T) {
uri: "http://localhost:25107/index",
api_token: data_manager_token,
}
+ unauth_prefix_req := &RequestTester{
+ method: "GET",
+ uri: "http://localhost:25107/index/" + TEST_HASH[0:3],
+ }
+ auth_prefix_req := &RequestTester{
+ method: "GET",
+ uri: "http://localhost:25107/index/" + TEST_HASH[0:3],
+ api_token: known_token,
+ }
+ superuser_prefix_req := &RequestTester{
+ method: "GET",
+ uri: "http://localhost:25107/index/" + TEST_HASH[0:3],
+ api_token: data_manager_token,
+ }
// ----------------------------
// enforce_permissions disabled
@@ -249,6 +285,14 @@ func TestIndexHandler(t *testing.T) {
PermissionError.HTTPCode,
response)
+ // unauthenticated /index/prefix request
+ // => PermissionError
+ response = IssueRequest(rest, unauth_prefix_req)
+ ExpectStatusCode(t,
+ "enforce_permissions off, unauthenticated /index/prefix request",
+ PermissionError.HTTPCode,
+ response)
+
// authenticated /index request, non-superuser
// => PermissionError
response = IssueRequest(rest, authenticated_req)
@@ -257,6 +301,14 @@ func TestIndexHandler(t *testing.T) {
PermissionError.HTTPCode,
response)
+ // authenticated /index/prefix request, non-superuser
+ // => PermissionError
+ response = IssueRequest(rest, auth_prefix_req)
+ ExpectStatusCode(t,
+ "enforce_permissions off, authenticated /index/prefix request, non-superuser",
+ PermissionError.HTTPCode,
+ response)
+
// authenticated /index request, superuser
// => PermissionError
response = IssueRequest(rest, superuser_req)
@@ -265,6 +317,14 @@ func TestIndexHandler(t *testing.T) {
PermissionError.HTTPCode,
response)
+ // superuser /index/prefix request
+ // => PermissionError
+ response = IssueRequest(rest, superuser_prefix_req)
+ ExpectStatusCode(t,
+ "enforce_permissions off, superuser /index/prefix request",
+ PermissionError.HTTPCode,
+ response)
+
// ---------------------------
// enforce_permissions enabled
// Only the superuser should be allowed to issue /index requests.
@@ -278,6 +338,14 @@ func TestIndexHandler(t *testing.T) {
PermissionError.HTTPCode,
response)
+ // unauthenticated /index/prefix request
+ // => PermissionError
+ response = IssueRequest(rest, unauth_prefix_req)
+ ExpectStatusCode(t,
+ "permissions on, unauthenticated /index/prefix request",
+ PermissionError.HTTPCode,
+ response)
+
// authenticated /index request, non-superuser
// => PermissionError
response = IssueRequest(rest, authenticated_req)
@@ -286,6 +354,14 @@ func TestIndexHandler(t *testing.T) {
PermissionError.HTTPCode,
response)
+ // authenticated /index/prefix request, non-superuser
+ // => PermissionError
+ response = IssueRequest(rest, auth_prefix_req)
+ ExpectStatusCode(t,
+ "permissions on, authenticated /index/prefix request, non-superuser",
+ PermissionError.HTTPCode,
+ response)
+
// superuser /index request
// => OK
response = IssueRequest(rest, superuser_req)
@@ -305,12 +381,7 @@ func TestIndexHandler(t *testing.T) {
// superuser /index/prefix request
// => OK
- response = IssueRequest(rest,
- &RequestTester{
- method: "GET",
- uri: "http://localhost:25107/index/" + TEST_HASH[0:3],
- api_token: data_manager_token,
- })
+ response = IssueRequest(rest, superuser_prefix_req)
ExpectStatusCode(t,
"permissions on, superuser request",
http.StatusOK,
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index 0fdf660..8219fc1 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -119,7 +119,8 @@ func main() {
&data_manager_token_file,
"data-manager-token-file",
"",
- "File with the API token used by the Data Manager. All DELETE requests or unqualified GET /index requests must carry this token.")
+ "File with the API token used by the Data Manager. All DELETE "+
+ "requests or GET /index requests must carry this token.")
flag.BoolVar(
&enforce_permissions,
"enforce-permissions",
@@ -129,27 +130,35 @@ func main() {
&listen,
"listen",
DEFAULT_ADDR,
- "interface on which to listen for requests, in the format ipaddr:port. e.g. -listen=10.0.1.24:8000. Use -listen=:port to listen on all network interfaces.")
+ "Interface on which to listen for requests, in the format "+
+ "ipaddr:port. e.g. -listen=10.0.1.24:8000. Use -listen=:port "+
+ "to listen on all network interfaces.")
flag.StringVar(
&permission_key_file,
"permission-key-file",
"",
- "File containing the secret key for generating and verifying permission signatures.")
+ "File containing the secret key for generating and verifying "+
+ "permission signatures.")
flag.IntVar(
&permission_ttl_sec,
"permission-ttl",
300,
- "Expiration time (in seconds) for newly generated permission signatures.")
+ "Expiration time (in seconds) for newly generated permission "+
+ "signatures.")
flag.BoolVar(
&serialize_io,
"serialize",
false,
- "If set, all read and write operations on local Keep volumes will be serialized.")
+ "If set, all read and write operations on local Keep volumes will "+
+ "be serialized.")
flag.StringVar(
&volumearg,
"volumes",
"",
- "Comma-separated list of directories to use for Keep volumes, e.g. -volumes=/var/keep1,/var/keep2. If empty or not supplied, Keep will scan mounted filesystems for volumes with a /keep top-level directory.")
+ "Comma-separated list of directories to use for Keep volumes, "+
+ "e.g. -volumes=/var/keep1,/var/keep2. If empty or not "+
+ "supplied, Keep will scan mounted filesystems for volumes "+
+ "with a /keep top-level directory.")
flag.Parse()
// Look for local keep volumes.
@@ -180,18 +189,20 @@ func main() {
}
// Initialize data manager token and permission key.
+ // If these tokens are specified but cannot be read,
+ // raise a fatal error.
if data_manager_token_file != "" {
if buf, err := ioutil.ReadFile(data_manager_token_file); err == nil {
data_manager_token = strings.TrimSpace(string(buf))
} else {
- log.Printf("reading data_manager_token: %s\n", err)
+ log.Fatalf("reading data manager token: %s\n", err)
}
}
if permission_key_file != "" {
if buf, err := ioutil.ReadFile(permission_key_file); err == nil {
PermissionSecret = bytes.TrimSpace(buf)
} else {
- log.Printf("reading data_manager_token: %s\n", err)
+ log.Fatalf("reading permission key: %s\n", err)
}
}
@@ -221,10 +232,13 @@ func main() {
func NewRESTRouter() *mux.Router {
rest := mux.NewRouter()
rest.HandleFunc(`/{hash:[0-9a-f]{32}}`, GetBlockHandler).Methods("GET", "HEAD")
- rest.HandleFunc(`/{hash:[0-9a-f]{32}}+A{signature:[0-9a-f]+}@{timestamp:[0-9a-f]+}`, GetBlockHandler).Methods("GET", "HEAD")
+ rest.HandleFunc(
+ `/{hash:[0-9a-f]{32}}+A{signature:[0-9a-f]+}@{timestamp:[0-9a-f]+}`,
+ GetBlockHandler).Methods("GET", "HEAD")
rest.HandleFunc(`/{hash:[0-9a-f]{32}}`, PutBlockHandler).Methods("PUT")
rest.HandleFunc(`/index`, IndexHandler).Methods("GET", "HEAD")
- rest.HandleFunc(`/index/{prefix:[0-9a-f]{0,32}}`, IndexHandler).Methods("GET", "HEAD")
+ rest.HandleFunc(
+ `/index/{prefix:[0-9a-f]{0,32}}`, IndexHandler).Methods("GET", "HEAD")
rest.HandleFunc(`/status.json`, StatusHandler).Methods("GET", "HEAD")
return rest
}
@@ -452,7 +466,7 @@ func GetBlock(hash string) ([]byte, error) {
// they should be sent directly to an event manager at high
// priority or logged as urgent problems.
//
- log.Printf("%s: checksum mismatch for request %s (actual hash %s)\n",
+ log.Printf("%s: checksum mismatch for request %s (actual %s)\n",
vol, hash, filehash)
return buf, CorruptError
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list