Skip to content

Commit

Permalink
fix: make sure to match servers even if they contain “regexp” chars (#9)
Browse files Browse the repository at this point in the history
This is a fix in the JWT token verification in both:

- the server verification which was using an uneeded regex in the
  `start_with?` parameter
- the path verification which was using the matching_server data
  inside a regexp without escaping the input data

I added a test case showcasing the issue.
  • Loading branch information
paulRbr authored Nov 25, 2024
1 parent 0a6d1b6 commit b608eef
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 14 deletions.
4 changes: 2 additions & 2 deletions proxy_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class ProxyServer < Sinatra::Base
target_url = request.fullpath[1..].gsub(":/", "://")

# Verify server is allowed
matching_server = @payload["servers"].find { |server| target_url.to_s.start_with?(/^#{server}/) }&.chomp("/")
matching_server = @payload["servers"].find { |server| target_url.to_s.start_with?(server) }&.chomp("/")

unless matching_server
halt 403, {error: "Server not allowed"}.to_json
Expand All @@ -91,7 +91,7 @@ class ProxyServer < Sinatra::Base

helpers do
def path_from_target_url(target_url, matching_server)
target_url.gsub(/^#{matching_server}/, "")
target_url.gsub(/^#{Regexp.escape(matching_server)}/, "")
end

def path_matches_pattern?(actual_path, pattern_path)
Expand Down
43 changes: 31 additions & 12 deletions spec/proxy_server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,18 @@ def expect_json_body(k, v)
.to_return(status: 200, body: {title: "updated title"}.to_json, headers: {})
stub_request(:post, "https://jsonplaceholder.typicode.com/posts")
.to_return(status: 201, body: {title: "foo", body: "bar", userId: 1}.to_json, headers: {})
stub_request(:get, "https://staging.bump.sh/api/v1/ping")
.with(
headers: {
'Accept'=>'*/*',
'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
'Cookie'=>'',
'Host'=>'staging.bump.sh',
'User-Agent'=>'Ruby',
'X-Foo'=>'bar'
})
.to_return(status: 200, body: "", headers: {})
["https://staging.bump.sh/api/v1/ping", "https://bump.sh/api/Custom+Api/v1/ping"].each do |server|
stub_request(:get, server)
.with(headers: {
'Accept' => '*/*',
'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
'Cookie'=>'',
'Host'=> URI.parse(server).host,
'User-Agent'=>'Ruby',
'X-Foo'=>'bar'
})
.to_return(status: 200, body: "", headers: {})
end
end

context "preflight request" do
Expand All @@ -99,7 +100,6 @@ def expect_json_body(k, v)
end

context "when server contains some path like /api/v1" do

let(:payload) do
{
"servers": [
Expand All @@ -118,6 +118,25 @@ def expect_json_body(k, v)
it "returns 200" do
expect(last_response.status).to eq(200)
end

context "when server contains path with regexp character" do
let(:payload) do
{
"servers": [
"https://bump.sh/api/Custom+Api/v1"
],
"verb": "GET",
"path": "/ping",
"exp": Time.now.to_i + 500
}
end

let(:target_url) { "https://bump.sh/api/Custom+Api/v1/ping"}

it "returns 200" do
expect(last_response.status).to eq(200)
end
end
end

it "returns cors headers" do
Expand Down

0 comments on commit b608eef

Please sign in to comment.