From 9a9a9c79e53cafa2feb9acdec3e90383c6eac41d Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 13:07:19 +0200 Subject: [PATCH 01/10] Send "Unauthorized" message body with 401 responses (refs #42) --- lib/remote_storage/swift.rb | 6 +++-- spec/swift/app_spec.rb | 46 +++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index 66f269a..aa6c3a3 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -24,12 +24,14 @@ module RemoteStorage return true if ["GET", "HEAD"].include?(request_method) && !listing end + server.halt 401, "Unauthorized" if token.empty? + authorizations = redis.smembers("authorizations:#{user}:#{token}") permission = directory_permission(authorizations, directory) - server.halt 401 unless permission + server.halt 401, "Unauthorized" unless permission if ["PUT", "DELETE"].include? request_method - server.halt 401 unless permission == "rw" + server.halt 401, "Unauthorized" unless permission == "rw" end end diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 29e5e4f..fd70aa4 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -173,6 +173,29 @@ describe "App" do purge_redis end + context "not authorized" do + + describe "with no token" do + it "says it's not authorized" do + delete "/phil/food/aguacate" + + last_response.status.must_equal 401 + last_response.body.must_equal "Unauthorized" + end + end + + describe "with wrong token" do + it "says it's not authorized" do + header "Authorization", "Bearer wrongtoken" + delete "/phil/food/aguacate" + + last_response.status.must_equal 401 + last_response.body.must_equal "Unauthorized" + end + end + + end + context "authorized" do before do redis.sadd "authorizations:phil:amarillo", [":rw"] @@ -248,6 +271,29 @@ describe "App" do purge_redis end + context "not authorized" do + + describe "without token" do + it "says it's not authorized" do + get "/phil/food/" + + last_response.status.must_equal 401 + last_response.body.must_equal "Unauthorized" + end + end + + describe "with wrong token" do + it "says it's not authorized" do + header "Authorization", "Bearer wrongtoken" + get "/phil/food/" + + last_response.status.must_equal 401 + last_response.body.must_equal "Unauthorized" + end + end + + end + context "authorized" do before do From 83d8f29a0404b96010173dc9dc5ad8fac31fbce9 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 13:09:05 +0200 Subject: [PATCH 02/10] Send "Conflict" message body with 409 responses (closes #409) --- lib/remote_storage/swift.rb | 2 +- spec/swift/app_spec.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index aa6c3a3..f1fad41 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -133,7 +133,7 @@ module RemoteStorage def put_data(user, directory, key, data, content_type) server.halt 400 if server.env["HTTP_CONTENT_RANGE"] - server.halt 409 if has_name_collision?(user, directory, key) + server.halt 409, "Conflict" if has_name_collision?(user, directory, key) existing_metadata = redis.hgetall redis_metadata_object_key(user, directory, key) url = url_for_key(user, directory, key) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index fd70aa4..0206d43 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -134,6 +134,7 @@ describe "App" do end last_response.status.must_equal 409 + last_response.body.must_equal "Conflict" metadata = redis.hgetall "rs:m:phil:food" metadata.must_be_empty From cc91b5c4cdfd6a26214776f39526caede37b143f Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 13:18:16 +0200 Subject: [PATCH 03/10] Send "Not Found" message body with 404 responses (refs #42) --- lib/remote_storage/swift.rb | 4 +- spec/swift/app_spec.rb | 77 +++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index f1fad41..fbab446 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -57,7 +57,7 @@ module RemoteStorage return res.body rescue RestClient::ResourceNotFound - server.halt 404 + server.halt 404, "Not Found" end def get_head_directory_listing(user, directory) @@ -187,7 +187,7 @@ module RemoteStorage server.halt 200 rescue RestClient::ResourceNotFound - server.halt 404 + server.halt 404, "Not Found" end private diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 0206d43..ecb0e4f 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -263,6 +263,16 @@ describe "App" do redis.smembers("rs:m:phil:/:items").must_be_empty end + + it "returns a 404 when item doesn't exist" do + raises_exception = ->(url, headers) { raise RestClient::ResourceNotFound.new } + RestClient.stub :delete, raises_exception do + delete "/phil/food/steak" + end + + last_response.status.must_equal 404 + last_response.body.must_equal "Not Found" + end end end @@ -313,6 +323,20 @@ describe "App" do end end + describe "data" do + + it "returns a 404 when data doesn't exist" do + raises_exception = ->(url, headers) { raise RestClient::ResourceNotFound.new } + RestClient.stub :get, raises_exception do + get "/phil/food/steak" + end + + last_response.status.must_equal 404 + last_response.body.must_equal "Not Found" + end + + end + describe "directory listings" do it "has an ETag in the header" do @@ -364,5 +388,58 @@ describe "App" do end end + + describe "HEAD requests" do + + before do + purge_redis + end + + context "not authorized" do + + describe "without token" do + it "says it's not authorized" do + head "/phil/food/camarones" + + last_response.status.must_equal 401 + last_response.body.must_be_empty + end + end + + describe "with wrong token" do + it "says it's not authorized" do + header "Authorization", "Bearer wrongtoken" + head "/phil/food/camarones" + + last_response.status.must_equal 401 + last_response.body.must_be_empty + end + end + + end + + context "authorized" do + + before do + redis.sadd "authorizations:phil:amarillo", [":rw"] + header "Authorization", "Bearer amarillo" + end + + describe "data" do + it "returns a 404 when data doesn't exist" do + raises_exception = ->(url, headers) { raise RestClient::ResourceNotFound.new } + RestClient.stub :head, raises_exception do + head "/phil/food/steak" + end + + last_response.status.must_equal 404 + last_response.body.must_be_empty + end + end + + end + + end + end From c897959029047409de2d8fea0e8c87eabb64d94a Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 18:49:20 +0200 Subject: [PATCH 04/10] Send "Precondition Failed" message body with 412 responses --- lib/remote_storage/swift.rb | 6 +-- spec/swift/app_spec.rb | 96 +++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index fbab446..2593fde 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -139,10 +139,10 @@ module RemoteStorage url = url_for_key(user, directory, key) if required_match = server.env["HTTP_IF_MATCH"] - server.halt 412 unless required_match == %Q("#{existing_metadata["e"]}") + server.halt 412, "Precondition Failed" unless required_match == %Q("#{existing_metadata["e"]}") end if server.env["HTTP_IF_NONE_MATCH"] == "*" - server.halt 412 unless existing_metadata.empty? + server.halt 412, "Precondition Failed" unless existing_metadata.empty? end res = do_put_request(url, data, content_type) @@ -178,7 +178,7 @@ module RemoteStorage existing_metadata = redis.hgetall "rs:m:#{user}:#{directory}/#{key}" if required_match = server.env["HTTP_IF_MATCH"] - server.halt 412 unless required_match == %Q("#{existing_metadata["e"]}") + server.halt 412, "Precondition Failed" unless required_match == %Q("#{existing_metadata["e"]}") end do_delete_request(url) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index ecb0e4f..298ab29 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -165,7 +165,82 @@ describe "App" do last_response.status.must_equal 400 end end + + describe "If-Match header" do + before do + put_stub = OpenStruct.new(headers: { + etag: "oldetag", + last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" + }) + + RestClient.stub :put, put_stub do + put "/phil/food/aguacate", "si" + end + end + + it "allows the request if the header matches the current ETag" do + header "If-Match", "\"oldetag\"" + + put_stub = OpenStruct.new(headers: { + etag: "newetag", + last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" + }) + + RestClient.stub :put, put_stub do + put "/phil/food/aguacate", "aye" + end + + last_response.status.must_equal 200 + last_response.headers["Etag"].must_equal "\"newetag\"" + end + + it "fails the request if the header does not match the current ETag" do + header "If-Match", "someotheretag" + + put "/phil/food/aguacate", "aye" + + last_response.status.must_equal 412 + last_response.body.must_equal "Precondition Failed" + end + end + + describe "If-None-Match header set to '*'" do + it "succeeds when the document doesn't exist yet" do + put_stub = OpenStruct.new(headers: { + etag: "someetag", + last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" + }) + + header "If-None-Match", "*" + + RestClient.stub :put, put_stub do + put "/phil/food/aguacate", "si" + end + + last_response.status.must_equal 200 + end + + it "fails the request if the document already exsits" do + put_stub = OpenStruct.new(headers: { + etag: "someetag", + last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" + }) + + RestClient.stub :put, put_stub do + put "/phil/food/aguacate", "si" + end + + header "If-None-Match", "*" + RestClient.stub :put, put_stub do + put "/phil/food/aguacate", "si" + end + + last_response.status.must_equal 412 + last_response.body.must_equal "Precondition Failed" + end + end end + end describe "DELETE requests" do @@ -273,6 +348,27 @@ describe "App" do last_response.status.must_equal 404 last_response.body.must_equal "Not Found" end + + describe "If-Match header" do + it "succeeds when the header matches the current ETag" do + header "If-Match", "\"bla\"" + + RestClient.stub :delete, "" do + delete "/phil/food/aguacate" + end + + last_response.status.must_equal 200 + end + + it "fails the request if it does not match the current ETag" do + header "If-Match", "someotheretag" + + delete "/phil/food/aguacate" + + last_response.status.must_equal 412 + last_response.body.must_equal "Precondition Failed" + end + end end end From ed8061b5945a75dc8e5b3d67aa453b97bbdab8ca Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 14:09:00 +0200 Subject: [PATCH 05/10] Use content type "application/ld+json" for directory listings --- lib/remote_storage/swift.rb | 2 +- spec/swift/app_spec.rb | 45 ++++++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index 66f269a..247f36c 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -67,7 +67,7 @@ module RemoteStorage def get_directory_listing(user, directory) etag = redis.hget "rs:m:#{user}:#{directory}/", "e" - server.headers["Content-Type"] = "application/json" + server.headers["Content-Type"] = "application/ld+json" none_match = (server.env["HTTP_IF_NONE_MATCH"] || "").split(",").map(&:strip) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 29e5e4f..dd7c752 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -90,7 +90,7 @@ describe "App" do get "phil/" last_response.status.must_equal 200 - last_response.content_type.must_equal "application/json" + last_response.content_type.must_equal "application/ld+json" content = JSON.parse(last_response.body) content["items"]["bamboo.txt"].wont_be_nil @@ -286,7 +286,7 @@ describe "App" do get "/phil/food/" last_response.status.must_equal 200 - last_response.content_type.must_equal "application/json" + last_response.content_type.must_equal "application/ld+json" content = JSON.parse(last_response.body) content["@context"].must_equal "http://remotestorage.io/spec/folder-description" @@ -306,7 +306,7 @@ describe "App" do get "phil/" last_response.status.must_equal 200 - last_response.content_type.must_equal "application/json" + last_response.content_type.must_equal "application/ld+json" content = JSON.parse(last_response.body) content["items"]["food/"].wont_be_nil @@ -317,5 +317,44 @@ describe "App" do end end + + describe "HEAD requests" do + + before do + purge_redis + end + + context "authorized" do + + before do + redis.sadd "authorizations:phil:amarillo", [":rw"] + header "Authorization", "Bearer amarillo" + + put_stub = OpenStruct.new(headers: { + etag: "bla", + last_modified: "Fri, 04 Mar 2016 12:20:18 GMT" + }) + + RestClient.stub :put, put_stub do + put "/phil/food/aguacate", "si" + put "/phil/food/camaron", "yummi" + put "/phil/food/desayunos/bolon", "wow" + end + end + + describe "directory listings" do + it "has the header information" do + get "/phil/food/" + + last_response.status.must_equal 200 + last_response.content_type.must_equal "application/ld+json" + last_response.headers["ETag"].must_equal "\"f9f85fbf5aa1fa378fd79ac8aa0a457d\"" + end + end + + end + + end + end From 23f0908f388fc1d48d2edc5fb4d135c3100de248 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 16:42:19 +0200 Subject: [PATCH 06/10] No need for Expires header anymore --- liquor-cabinet.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/liquor-cabinet.rb b/liquor-cabinet.rb index 084974a..c350c90 100644 --- a/liquor-cabinet.rb +++ b/liquor-cabinet.rb @@ -72,7 +72,6 @@ class LiquorCabinet < Sinatra::Base 'Access-Control-Expose-Headers' => 'ETag, Content-Length' headers['Access-Control-Allow-Origin'] = env["HTTP_ORIGIN"] if env["HTTP_ORIGIN"] headers['Cache-Control'] = 'no-cache' - headers['Expires'] = '0' @user, @key = params[:user], params[:key] @directory = params[:splat] && params[:splat].first || "" From 16e51038b10aa8d2ed7c2cf3fd2bb9aaece7c882 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 16:43:50 +0200 Subject: [PATCH 07/10] Send ETag of deleted item in header --- lib/remote_storage/swift.rb | 1 + spec/swift/app_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/lib/remote_storage/swift.rb b/lib/remote_storage/swift.rb index 247f36c..d2b1aa7 100644 --- a/lib/remote_storage/swift.rb +++ b/lib/remote_storage/swift.rb @@ -183,6 +183,7 @@ module RemoteStorage delete_metadata_objects(user, directory, key) delete_dir_objects(user, directory) + server.headers["Etag"] = %Q("#{existing_metadata["e"]}") server.halt 200 rescue RestClient::ResourceNotFound server.halt 404 diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index dd7c752..264fc59 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -239,6 +239,14 @@ describe "App" do redis.smembers("rs:m:phil:/:items").must_be_empty end + + it "responds with the ETag of the deleted item in the haeder" do + RestClient.stub :delete, "" do + delete "/phil/food/aguacate" + end + + last_response.headers["ETag"].must_equal "\"bla\"" + end end end From 576e4a9afbba28e32ada892e20fab39ecc4373ae Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 16:44:44 +0200 Subject: [PATCH 08/10] Specs for response headers on GET requests --- spec/swift/app_spec.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 264fc59..10bc259 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -274,6 +274,28 @@ describe "App" do end end + describe "data" do + + it "has the required response headers" do + get_stub = OpenStruct.new(body: "si", headers: { + etag: "0815etag", + last_modified: "Fri, 04 Mar 2016 12:20:18 GMT", + content_type: "text/plain; charset=utf-8", + content_length: 2 + }) + + RestClient.stub :get, get_stub do + get "/phil/food/aguacate" + end + + last_response.status.must_equal 200 + last_response.headers["ETag"].must_equal "\"0815etag\"" + last_response.headers["Cache-Control"].must_equal "no-cache" + last_response.headers["Content-Type"].must_equal "text/plain; charset=utf-8" + end + + end + describe "directory listings" do it "has an ETag in the header" do @@ -283,6 +305,13 @@ describe "App" do last_response.headers["ETag"].must_equal "\"f9f85fbf5aa1fa378fd79ac8aa0a457d\"" end + it "has a Cache-Control in the header" do + get "/phil/food/" + + last_response.status.must_equal 200 + last_response.headers["Cache-Control"].must_equal "no-cache" + end + it "responds with 304 when IF_NONE_MATCH header contains the ETag" do header "If-None-Match", "\"f9f85fbf5aa1fa378fd79ac8aa0a457d\"" get "/phil/food/" From a349db524308acc3c4dd157dd33598224c4d76c8 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Thu, 2 Jun 2016 16:45:17 +0200 Subject: [PATCH 09/10] Spec for directory listing of non-existing directories --- spec/swift/app_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 10bc259..87dcc37 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -350,6 +350,16 @@ describe "App" do content["items"]["food/"]["ETag"].must_equal "f9f85fbf5aa1fa378fd79ac8aa0a457d" end + it "responds with 200 and empty object when directory doesn't exist" do + get "phil/some-non-existing-dir/" + + last_response.status.must_equal 200 + last_response.content_type.must_equal "application/ld+json" + + content = JSON.parse(last_response.body) + content["items"].must_equal({}) + end + end end From c1d60586ec034da518ce659d90cbb1688a554559 Mon Sep 17 00:00:00 2001 From: Garret Alfert Date: Fri, 3 Jun 2016 01:08:52 +0200 Subject: [PATCH 10/10] Improve some spec wordings --- spec/swift/app_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/swift/app_spec.rb b/spec/swift/app_spec.rb index 4df2ad0..a30c52e 100644 --- a/spec/swift/app_spec.rb +++ b/spec/swift/app_spec.rb @@ -427,9 +427,9 @@ describe "App" do end end - describe "data" do + describe "documents" do - it "has the required response headers" do + it "returns the required response headers" do get_stub = OpenStruct.new(body: "si", headers: { etag: "0815etag", last_modified: "Fri, 04 Mar 2016 12:20:18 GMT", @@ -461,14 +461,14 @@ describe "App" do describe "directory listings" do - it "has an ETag in the header" do + it "returns the correct ETag header" do get "/phil/food/" last_response.status.must_equal 200 last_response.headers["ETag"].must_equal "\"f9f85fbf5aa1fa378fd79ac8aa0a457d\"" end - it "has a Cache-Control in the header" do + it "returns a Cache-Control header with value 'no-cache'" do get "/phil/food/" last_response.status.must_equal 200 @@ -513,7 +513,7 @@ describe "App" do content["items"]["food/"]["ETag"].must_equal "f9f85fbf5aa1fa378fd79ac8aa0a457d" end - it "responds with 200 and empty object when directory doesn't exist" do + it "responds with an empty directory liting when directory doesn't exist" do get "phil/some-non-existing-dir/" last_response.status.must_equal 200 @@ -576,7 +576,7 @@ describe "App" do end describe "directory listings" do - it "has the header information" do + it "returns the correct header information" do get "/phil/food/" last_response.status.must_equal 200 @@ -585,8 +585,8 @@ describe "App" do end end - describe "data" do - it "returns a 404 when data doesn't exist" do + describe "documents" do + it "returns a 404 when the document doesn't exist" do raises_exception = ->(url, headers) { raise RestClient::ResourceNotFound.new } RestClient.stub :head, raises_exception do head "/phil/food/steak"