From c3fb2e402c50b0c526c697d8035cf44692ed1e03 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 16 Oct 2024 14:10:33 -0700 Subject: [PATCH 1/6] when generating headers in the client, remove query string params from the path --- src/kemal-hmac/client.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kemal-hmac/client.cr b/src/kemal-hmac/client.cr index f24c2a3..9cc18ea 100644 --- a/src/kemal-hmac/client.cr +++ b/src/kemal-hmac/client.cr @@ -22,7 +22,7 @@ module Kemal::Hmac # :return: a Hash of the HMAC headers def generate_headers(path : String) : Hash(String, String) timestamp = Time::Format::ISO_8601_DATE_TIME.format(Time.utc) - hmac_token = Kemal::Hmac::Token.new(@client, path, timestamp, @algorithm).hexdigest(@secret) + hmac_token = Kemal::Hmac::Token.new(@client, path.split("?").first, timestamp, @algorithm).hexdigest(@secret) return { HMAC_CLIENT_HEADER => @client, From b98903f4f8ef8a772284600bbd37a60bd23630bf Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 16 Oct 2024 14:10:37 -0700 Subject: [PATCH 2/6] add tests --- spec/kemal-hmac/kemal-hmac_spec.cr | 50 ++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/spec/kemal-hmac/kemal-hmac_spec.cr b/spec/kemal-hmac/kemal-hmac_spec.cr index 26281b0..9409170 100644 --- a/spec/kemal-hmac/kemal-hmac_spec.cr +++ b/spec/kemal-hmac/kemal-hmac_spec.cr @@ -41,6 +41,56 @@ describe "Kemal::Hmac" do context.kemal_authorized_client?.should eq(client) end + it "uses a custom handler and correct HMAC auth and the request flows through successfully with query string params" do + client = "valid-octo-client" + hmac_handler = SpecAuthHandler.new( + hmac_secrets: {client => ["octo-secret-blue", "octo-secret-green"]}, + ) + + hmac_client = Kemal::Hmac::Client.new(client, "octo-secret-green", "SHA256") + headers = hmac_client.generate_headers("/api") + + request = HTTP::Request.new( + "GET", + "/api?foo=bar&moon=star", + headers: HTTP::Headers{ + "hmac-client" => headers["hmac-client"], + "hmac-timestamp" => headers["hmac-timestamp"], + "hmac-token" => headers["hmac-token"], + }, + ) + + io, context = create_request_and_return_io_and_context(hmac_handler, request) + response = HTTP::Client::Response.from_io(io, decompress: false) + response.status_code.should eq 404 + context.kemal_authorized_client?.should eq(client) + end + + it "uses a custom handler and correct HMAC auth and the request flows through successfully with query string params incorrecty in the client" do + client = "valid-octo-client" + hmac_handler = SpecAuthHandler.new( + hmac_secrets: {client => ["octo-secret-blue", "octo-secret-green"]}, + ) + + hmac_client = Kemal::Hmac::Client.new(client, "octo-secret-green", "SHA256") + headers = hmac_client.generate_headers("/api?cat=dog&moon=star&q=1") + + request = HTTP::Request.new( + "GET", + "/api?foo=bar&moon=star", + headers: HTTP::Headers{ + "hmac-client" => headers["hmac-client"], + "hmac-timestamp" => headers["hmac-timestamp"], + "hmac-token" => headers["hmac-token"], + }, + ) + + io, context = create_request_and_return_io_and_context(hmac_handler, request) + response = HTTP::Client::Response.from_io(io, decompress: false) + response.status_code.should eq 404 + context.kemal_authorized_client?.should eq(client) + end + it "uses a custom handler and explicit hmac algo successfully" do client = "valid-octo-client" hmac_handler = SpecAuthHandler.new( From 70b9e851faab3015ba5a5af8b4c47912c018b2c7 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 16 Oct 2024 14:14:16 -0700 Subject: [PATCH 3/6] acceptance tests for qsp --- script/acceptance_tests/tests.cr | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/script/acceptance_tests/tests.cr b/script/acceptance_tests/tests.cr index 2e0c60f..881568a 100644 --- a/script/acceptance_tests/tests.cr +++ b/script/acceptance_tests/tests.cr @@ -18,10 +18,24 @@ describe "crest" do response.body.should contain "Hi, my_crest_client!" end + it "successfully sends a request with HMAC auth with the crest client to the crest-client endpoint with query string params" do + client = Kemal::Hmac::Client.new("my_crest_client", "my_secret") + + path = "/crest-client" + + response = Crest.get( + "http://localhost:3000#{path}", + headers: client.generate_headers(path) + ) + + response.status_code.should eq 200 + response.body.should contain "Hi, my_crest_client!" + end + it "fails to send a request with HMAC auth with the crest client to the crest-client endpoint" do client = Kemal::Hmac::Client.new("my_crest_client", "incorrect_secret") - path = "/crest-client" + path = "/crest-client?foo=bar&q=baz&operation=1/2" begin Crest.get( @@ -52,6 +66,22 @@ describe "http/client" do response.body.should contain "Hi, my_standard_client!" end + it "successfully sends a request with HMAC auth with the http-client to the http-client endpoint with query string params" do + client = Kemal::Hmac::Client.new("my_standard_client", "my_secret_2") + + path = "/http-client?foo=bar&q=baz&operation=1/2" + + headers = HTTP::Headers.new + client.generate_headers(path).each do |key, value| + headers.add(key, value) + end + + response = HTTP::Client.get("http://localhost:3000#{path}", headers: headers) + + response.status_code.should eq 200 + response.body.should contain "Hi, my_standard_client!" + end + it "fails to send a request with HMAC auth with the http-client to the http-client endpoint" do client = Kemal::Hmac::Client.new("my_standard_client", "bad_secret") From 71b74ead5c3d247ec69440250c2ce6356594bd03 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 16 Oct 2024 14:38:33 -0700 Subject: [PATCH 4/6] test all HTTP methods --- script/acceptance_tests/server.cr | 31 +++++++++++++ script/acceptance_tests/tests.cr | 72 +++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/script/acceptance_tests/server.cr b/script/acceptance_tests/server.cr index a17a3e6..81edf7d 100644 --- a/script/acceptance_tests/server.cr +++ b/script/acceptance_tests/server.cr @@ -15,4 +15,35 @@ get "/crest-client" do |env| "Hi, %s! You sent a request that was successfully verified with HMAC auth for the crest-client" % env.kemal_authorized_client? end +post "/submit" do |env| + "Hi, %s! I got your POST request" % env.kemal_authorized_client? +end + +# catch-all routes for all other requests and methods +# ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS", "TRACE", "CONNECT"] +get "/catch-all" do |env| + "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? +end + +post "/catch-all" do |env| + "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? +end + +put "/catch-all" do |env| + "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? +end + +patch "/catch-all" do |env| + "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? +end + +delete "/catch-all" do |env| + "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? +end + +options "/catch-all" do |env| + "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? +end + + Kemal.run diff --git a/script/acceptance_tests/tests.cr b/script/acceptance_tests/tests.cr index 881568a..5cb72ed 100644 --- a/script/acceptance_tests/tests.cr +++ b/script/acceptance_tests/tests.cr @@ -3,6 +3,32 @@ require "crest" require "http/client" require "spec" +describe "All HTTP Methods" do + ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"].each do |method| + it "successfully validates a #{method} request" do + client = Kemal::Hmac::Client.new("my_standard_client", "my_secret_2") + + path = "/catch-all" + + headers = HTTP::Headers.new + client.generate_headers(path).each do |key, value| + headers.add(key, value) + end + + response = HTTP::Client.get("http://localhost:3000#{path}", headers: headers) + + request = HTTP::Request.new( + method, + "/api", + headers: headers, + ) + + response.status_code.should eq 200 + response.body.should contain "Hi, my_standard_client!" + end + end +end + describe "crest" do it "successfully sends a request with HMAC auth with the crest client to the crest-client endpoint" do client = Kemal::Hmac::Client.new("my_crest_client", "my_secret") @@ -47,6 +73,36 @@ describe "crest" do ex.response.body.should contain "Unauthorized: HMAC token does not match" end end + + it "fails to send a request with HMAC auth to an endpoint using the POST HTTP verb" do + client = Kemal::Hmac::Client.new("my_crest_client", "incorrect_secret") + + path = "/submit" + + begin + Crest.post( + "http://localhost:3000#{path}", + headers: client.generate_headers(path) + ) + rescue ex : Crest::Unauthorized + ex.response.status_code.should eq 401 + ex.response.body.should contain "Unauthorized: HMAC token does not match" + end + end + + it "successfully sends a request with HMAC auth with the crest client an endpoint using the POST HTTP verb" do + client = Kemal::Hmac::Client.new("my_crest_client", "my_secret") + + path = "/submit" + + response = Crest.post( + "http://localhost:3000#{path}", + headers: client.generate_headers(path) + ) + + response.status_code.should eq 200 + response.body.should contain "Hi, my_crest_client! I got your POST request" + end end describe "http/client" do @@ -66,6 +122,22 @@ describe "http/client" do response.body.should contain "Hi, my_standard_client!" end + it "successfully sends a request with HMAC auth with the http-client to an endpoint using the POST HTTP verb" do + client = Kemal::Hmac::Client.new("my_standard_client", "my_secret_2") + + path = "/submit" + + headers = HTTP::Headers.new + client.generate_headers(path).each do |key, value| + headers.add(key, value) + end + + response = HTTP::Client.post("http://localhost:3000#{path}", headers: headers) + + response.status_code.should eq 200 + response.body.should contain "Hi, my_standard_client! I got your POST request" + end + it "successfully sends a request with HMAC auth with the http-client to the http-client endpoint with query string params" do client = Kemal::Hmac::Client.new("my_standard_client", "my_secret_2") From 088efdea7770878ca852589aac7c7feb645a7ba7 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 16 Oct 2024 14:45:43 -0700 Subject: [PATCH 5/6] lint --- script/acceptance_tests/server.cr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/script/acceptance_tests/server.cr b/script/acceptance_tests/server.cr index 81edf7d..445f639 100644 --- a/script/acceptance_tests/server.cr +++ b/script/acceptance_tests/server.cr @@ -20,7 +20,7 @@ post "/submit" do |env| end # catch-all routes for all other requests and methods -# ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS", "TRACE", "CONNECT"] +# ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS", "TRACE", "CONNECT"] get "/catch-all" do |env| "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? end @@ -45,5 +45,4 @@ options "/catch-all" do |env| "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? end - Kemal.run From 22ec0d304054bfd9bdce6496b1abb066d0a7601d Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 16 Oct 2024 14:46:03 -0700 Subject: [PATCH 6/6] comment fix --- script/acceptance_tests/server.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/acceptance_tests/server.cr b/script/acceptance_tests/server.cr index 445f639..8f3eb2d 100644 --- a/script/acceptance_tests/server.cr +++ b/script/acceptance_tests/server.cr @@ -20,7 +20,7 @@ post "/submit" do |env| end # catch-all routes for all other requests and methods -# ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS", "TRACE", "CONNECT"] +# ["GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS"] get "/catch-all" do |env| "Hi, %s! Welcome to catch-all" % env.kemal_authorized_client? end