[ARVADOS] updated: d269c98c322e0a65ef36730e6a57c98a1ed754fa
git at public.curoverse.com
git at public.curoverse.com
Sat May 24 00:48:36 EDT 2014
Summary of changes:
sdk/python/test_keep_client.py | 51 ++++++++++++++++++++++++++++-------------
services/keep/src/keep/keep.go | 21 +++++++++++++----
services/keep/src/keep/perms.go | 4 ++--
3 files changed, 54 insertions(+), 22 deletions(-)
via d269c98c322e0a65ef36730e6a57c98a1ed754fa (commit)
from 521457373a8f1e46f44a43311be2d9242ad5d0a9 (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 d269c98c322e0a65ef36730e6a57c98a1ed754fa
Author: Tim Pierce <twp at curoverse.com>
Date: Sat May 24 00:44:06 2014 -0400
2755: incorporate code review.
* Unit tests cover all permutations of signature/authorization when
--enforce-permissions=false
* Keep is more forgiving about the structure of locators, permits
locator hints of unknown type (as long as they begin with an uppercase
letter)
* Keep delivers 400 Bad Request for requests that do not match any
route, or are lexically invalid. 404 Not Found only for requests with
a syntactically valid hash not found on disk.
Refs #2755.
diff --git a/sdk/python/test_keep_client.py b/sdk/python/test_keep_client.py
index 8fac34d..f863ad3 100644
--- a/sdk/python/test_keep_client.py
+++ b/sdk/python/test_keep_client.py
@@ -93,7 +93,7 @@ class KeepPermissionTestCase(unittest.TestCase):
'foo',
'wrong content from Keep.get(md5("foo"))')
- # With Keep permissions enabled, a GET request without a locator will fail.
+ # With Keep permissions enabled, a GET request without a signature will fail.
bar_locator = arvados.Keep.put('bar')
self.assertRegexpMatches(
bar_locator,
@@ -113,6 +113,12 @@ class KeepPermissionTestCase(unittest.TestCase):
# but not --enforce-permissions (i.e. generate signatures on PUT
# requests, but do not require them for GET requests)
#
+# All of these requests should succeed when permissions are optional:
+# * authenticated request, signed locator
+# * authenticated request, unsigned locator
+# * unauthenticated request, signed locator
+# * unauthenticated request, unsigned locator
+
class KeepOptionalPermission(unittest.TestCase):
@classmethod
def setUpClass(cls):
@@ -129,39 +135,52 @@ class KeepOptionalPermission(unittest.TestCase):
run_test_server.stop()
run_test_server.stop_keep()
- def test_KeepBasicRWTest(self):
+ def test_KeepAuthenticatedSignedTest(self):
run_test_server.authorize_with('active')
- foo_locator = arvados.Keep.put('foo')
+ signed_locator = arvados.Keep.put('foo')
self.assertRegexpMatches(
- foo_locator,
+ signed_locator,
r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
- 'invalid locator from Keep.put("foo"): ' + foo_locator)
- self.assertEqual(arvados.Keep.get(foo_locator),
+ 'invalid locator from Keep.put("foo"): ' + signed_locator)
+ self.assertEqual(arvados.Keep.get(signed_locator),
'foo',
'wrong content from Keep.get(md5("foo"))')
- def test_KeepUnsignedLocatorTest(self):
- # Since --enforce-permissions is not in effect, GET requests
- # do not require signatures.
+ def test_KeepAuthenticatedUnsignedTest(self):
run_test_server.authorize_with('active')
- foo_locator = arvados.Keep.put('foo')
+ signed_locator = arvados.Keep.put('foo')
self.assertRegexpMatches(
- foo_locator,
+ signed_locator,
r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
- 'invalid locator from Keep.put("foo"): ' + foo_locator)
+ 'invalid locator from Keep.put("foo"): ' + signed_locator)
self.assertEqual(arvados.Keep.get("acbd18db4cc2f85cedef654fccc4a4d8"),
'foo',
'wrong content from Keep.get(md5("foo"))')
- def test_KeepUnauthenticatedTest(self):
+ def test_KeepUnauthenticatedSignedTest(self):
# Since --enforce-permissions is not in effect, GET requests
# need not be authenticated.
run_test_server.authorize_with('active')
- foo_locator = arvados.Keep.put('foo')
+ signed_locator = arvados.Keep.put('foo')
self.assertRegexpMatches(
- foo_locator,
+ signed_locator,
r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
- 'invalid locator from Keep.put("foo"): ' + foo_locator)
+ 'invalid locator from Keep.put("foo"): ' + signed_locator)
+
+ del arvados.config.settings()["ARVADOS_API_TOKEN"]
+ self.assertEqual(arvados.Keep.get(signed_locator),
+ 'foo',
+ 'wrong content from Keep.get(md5("foo"))')
+
+ def test_KeepUnauthenticatedUnsignedTest(self):
+ # Since --enforce-permissions is not in effect, GET requests
+ # need not be authenticated.
+ run_test_server.authorize_with('active')
+ signed_locator = arvados.Keep.put('foo')
+ self.assertRegexpMatches(
+ signed_locator,
+ r'^acbd18db4cc2f85cedef654fccc4a4d8\+3\+A[a-f0-9]+@[a-f0-9]+$',
+ 'invalid locator from Keep.put("foo"): ' + signed_locator)
del arvados.config.settings()["ARVADOS_API_TOKEN"]
self.assertEqual(arvados.Keep.get("acbd18db4cc2f85cedef654fccc4a4d8"),
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index 1f80678..b0112e2 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -70,6 +70,7 @@ type KeepError struct {
}
var (
+ BadRequestError = &KeepError{400, "Bad Request"}
CollisionError = &KeepError{400, "Collision"}
MD5Error = &KeepError{401, "MD5 Failure"}
PermissionError = &KeepError{401, "Permission denied"}
@@ -268,11 +269,13 @@ func main() {
//
func MakeRESTRouter() *mux.Router {
rest := mux.NewRouter()
+
rest.HandleFunc(
`/{hash:[0-9a-f]{32}}`, GetBlockHandler).Methods("GET", "HEAD")
rest.HandleFunc(
`/{hash:[0-9a-f]{32}}+{hints}`,
GetBlockHandler).Methods("GET", "HEAD")
+
rest.HandleFunc(`/{hash:[0-9a-f]{32}}`, PutBlockHandler).Methods("PUT")
// For IndexHandler we support:
@@ -291,9 +294,18 @@ func MakeRESTRouter() *mux.Router {
rest.HandleFunc(
`/index/{prefix:[0-9a-f]{0,32}}`, IndexHandler).Methods("GET", "HEAD")
rest.HandleFunc(`/status.json`, StatusHandler).Methods("GET", "HEAD")
+
+ // Any request which does not match any of these routes gets
+ // 400 Bad Request.
+ rest.NotFoundHandler = http.HandlerFunc(BadRequestHandler)
+
return rest
}
+func BadRequestHandler(w http.ResponseWriter, r *http.Request) {
+ http.Error(w, BadRequestError.Error(), BadRequestError.HTTPCode)
+}
+
// FindKeepVolumes
// Returns a list of Keep volumes mounted on this system.
//
@@ -340,14 +352,15 @@ func GetBlockHandler(resp http.ResponseWriter, req *http.Request) {
for _, hint := range strings.Split(hints, "+") {
if match, _ := regexp.MatchString("^[[:digit:]]+$", hint); match {
// Server ignores size hints
- } else if match, _ := regexp.MatchString("^K([[:alnum:]]+)$", hint); match {
- // Server ignores location hints
} else if m := signature_pat.FindStringSubmatch(hint); m != nil {
signature = m[1]
timestamp = m[2]
+ } else if match, _ := regexp.MatchString("^[[:upper:]]", hint); match {
+ // Any unknown hint that starts with an uppercase letter is
+ // presumed to be valid and ignored, to permit forward compatibility.
} else {
- // Not a valid locator: return 404
- http.Error(resp, NotFoundError.Error(), NotFoundError.HTTPCode)
+ // Unknown format; not a valid locator.
+ http.Error(resp, BadRequestError.Error(), BadRequestError.HTTPCode)
return
}
}
diff --git a/services/keep/src/keep/perms.go b/services/keep/src/keep/perms.go
index f7caca4..1438155 100644
--- a/services/keep/src/keep/perms.go
+++ b/services/keep/src/keep/perms.go
@@ -83,10 +83,10 @@ func SignLocator(blob_locator string, api_token string, expiry time.Time) string
// VerifySignature returns true if the signature on the signed_locator
// can be verified using the given api_token.
func VerifySignature(signed_locator string, api_token string) bool {
- if re, err := regexp.Compile(`^([a-f0-9]+(\+[0-9]+)?)\+A(.*)@(.*)$`); err == nil {
+ if re, err := regexp.Compile(`^([a-f0-9]{32}(\+[0-9]+)?).*\+A[[:xdigit:]]+@([[:xdigit:]]{8})`); err == nil {
if matches := re.FindStringSubmatch(signed_locator); matches != nil {
blob_locator := matches[1]
- timestamp_hex := matches[4]
+ timestamp_hex := matches[3]
if expire_ts, err := ParseHexTimestamp(timestamp_hex); err == nil {
// Fail signatures with expired timestamps.
if expire_ts.Before(time.Now()) {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list