[ARVADOS] updated: 69441aa2a477099731f36f6e7995fc2ec85c8c86
git at public.curoverse.com
git at public.curoverse.com
Wed Oct 15 17:18:44 EDT 2014
Summary of changes:
services/keepstore/handler_test.go | 113 ++++++++++++-------------------------
services/keepstore/handlers.go | 17 +++---
2 files changed, 43 insertions(+), 87 deletions(-)
via 69441aa2a477099731f36f6e7995fc2ec85c8c86 (commit)
via adbea418d67bfe3b30a305c43d22b858d5a81e92 (commit)
from 8cfc0583424956ca6b2a3f1f2efcf751e185aa3a (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 69441aa2a477099731f36f6e7995fc2ec85c8c86
Merge: 8cfc058 adbea41
Author: mishaz <misha at curoverse.com>
Date: Wed Oct 15 21:18:25 2014 +0000
Merge branch '4197-remove-enforce-permissions-check-from-index-handler'
Closes #4197
commit adbea418d67bfe3b30a305c43d22b858d5a81e92
Author: mishaz <misha at curoverse.com>
Date: Tue Oct 14 22:11:40 2014 +0000
Modified IndexHandler to match TrashHandler and PullHandler
* No longer checks for the enforce-permissions flag.
* Still checks for DataManager auth token.
* The HTTP error returned when we don't find the DataManager auth token is now Unauthorized instead of Forbidden.
Modified tests to check for new behavior.
diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 0829ce9..ca60915 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -224,21 +224,15 @@ func TestPutHandler(t *testing.T) {
}
// Test /index requests:
-// - 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
+// - unauthenticated /index request
+// - unauthenticated /index/prefix request
+// - authenticated /index request | non-superuser
+// - authenticated /index/prefix request | non-superuser
+// - authenticated /index request | superuser
+// - authenticated /index/prefix request | superuser
//
// The only /index requests that should succeed are those issued by the
-// superuser when enforce_permissions = true.
+// superuser. They should pass regardless of the value of enforce_permissions.
//
func TestIndexHandler(t *testing.T) {
defer teardown()
@@ -289,95 +283,58 @@ func TestIndexHandler(t *testing.T) {
api_token: data_manager_token,
}
- // ----------------------------
- // enforce_permissions disabled
- // All /index requests should fail.
- enforce_permissions = false
+ // -------------------------------------------------------------
+ // Only the superuser should be allowed to issue /index requests.
+
+ // ---------------------------
+ // enforce_permissions enabled
+ // This setting should not affect tests passing.
+ enforce_permissions = true
// unauthenticated /index request
- // => PermissionError
+ // => UnauthorizedError
response := IssueRequest(rest, unauthenticated_req)
ExpectStatusCode(t,
- "enforce_permissions off, unauthenticated request",
- PermissionError.HTTPCode,
+ "enforce_permissions on, unauthenticated request",
+ UnauthorizedError.HTTPCode,
response)
// unauthenticated /index/prefix request
- // => PermissionError
+ // => UnauthorizedError
response = IssueRequest(rest, unauth_prefix_req)
ExpectStatusCode(t,
- "enforce_permissions off, unauthenticated /index/prefix request",
- PermissionError.HTTPCode,
+ "permissions on, unauthenticated /index/prefix request",
+ UnauthorizedError.HTTPCode,
response)
// authenticated /index request, non-superuser
- // => PermissionError
+ // => UnauthorizedError
response = IssueRequest(rest, authenticated_req)
ExpectStatusCode(t,
- "enforce_permissions off, authenticated request, non-superuser",
- PermissionError.HTTPCode,
+ "permissions on, authenticated request, non-superuser",
+ UnauthorizedError.HTTPCode,
response)
// authenticated /index/prefix request, non-superuser
- // => PermissionError
+ // => UnauthorizedError
response = IssueRequest(rest, auth_prefix_req)
ExpectStatusCode(t,
- "enforce_permissions off, authenticated /index/prefix request, non-superuser",
- PermissionError.HTTPCode,
+ "permissions on, authenticated /index/prefix request, non-superuser",
+ UnauthorizedError.HTTPCode,
response)
- // authenticated /index request, superuser
- // => PermissionError
+ // superuser /index request
+ // => OK
response = IssueRequest(rest, superuser_req)
ExpectStatusCode(t,
- "enforce_permissions off, superuser request",
- 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.
- enforce_permissions = true
-
- // unauthenticated /index request
- // => PermissionError
- response = IssueRequest(rest, unauthenticated_req)
- ExpectStatusCode(t,
- "enforce_permissions on, unauthenticated request",
- 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)
- ExpectStatusCode(t,
- "permissions on, authenticated request, non-superuser",
- PermissionError.HTTPCode,
+ "permissions on, superuser request",
+ http.StatusOK,
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)
+ // ----------------------------
+ // enforce_permissions disabled
+ // Valid Request should still pass.
+ enforce_permissions = false
// superuser /index request
// => OK
@@ -387,6 +344,8 @@ func TestIndexHandler(t *testing.T) {
http.StatusOK,
response)
+
+
expected := `^` + TEST_HASH + `\+\d+ \d+\n` +
TEST_HASH_2 + `\+\d+ \d+\n$`
match, _ := regexp.MatchString(expected, response.Body.String())
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 1ef9915..27d1e90 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -244,18 +244,15 @@ func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
// A HandleFunc to address /index and /index/{prefix} requests.
//
func IndexHandler(resp http.ResponseWriter, req *http.Request) {
- prefix := mux.Vars(req)["prefix"]
-
- // Only the data manager may issue /index requests,
- // and only if enforce_permissions is enabled.
- // All other requests return 403 Forbidden.
- api_token := GetApiToken(req)
- if !enforce_permissions ||
- api_token == "" ||
- data_manager_token != api_token {
- http.Error(resp, PermissionError.Error(), PermissionError.HTTPCode)
+ // Reject unauthorized requests.
+ if !IsDataManagerToken(GetApiToken(req)) {
+ http.Error(resp, UnauthorizedError.Error(), UnauthorizedError.HTTPCode)
+ log.Printf("%s %s: %s\n", req.Method, req.URL, UnauthorizedError.Error())
return
}
+
+ prefix := mux.Vars(req)["prefix"]
+
var index string
for _, vol := range KeepVM.Volumes() {
index = index + vol.Index(prefix)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list