From 725c6ccb7cf7efc51b4394f9828585eea9c379d9 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Mon, 15 Apr 2019 19:19:38 +1000 Subject: [PATCH] fix: correct logic for filtering ui/api requests --- lib/rack/pact_broker/ui_request_filter.rb | 43 +++++++++++++------ .../pact_broker/ui_request_filter_spec.rb | 15 +++++-- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/lib/rack/pact_broker/ui_request_filter.rb b/lib/rack/pact_broker/ui_request_filter.rb index addeffeaf..bf1fa6896 100644 --- a/lib/rack/pact_broker/ui_request_filter.rb +++ b/lib/rack/pact_broker/ui_request_filter.rb @@ -1,36 +1,53 @@ # Decides whether this is a request for the UI or a request for the API +# This is only needed so that the correct authentication method is applied (UI or API auth) module Rack module PactBroker class UIRequestFilter + WEB_EXTENSIONS = %w[.js .woff .woff2 .css .png .html .map .ttf .ico].freeze + API_CONTENT_TYPES = %w[application/hal+json application/json text/csv application/yaml].freeze + def initialize app @app = app end def call env - if request_for_ui_resource? env - @app.call(env) - else + if request_for_api(env) || (accept_all(env) && !is_web_extension(env)) + # send the request on to the next app in the Rack::Cascade [404, {},[]] + else + @app.call(env) end end private - def request_for_ui_resource? env - request_for_file?(env) || accepts_html?(env) + def body_is_json(env) + env['CONTENT_TYPE'] && env['CONTENT_TYPE'].include?("json") end - def request_for_file?(env) - if last_segment = env['PATH_INFO'].split("/").last - last_segment.include?(".") - else - false - end + def request_for_api(env) + accepts_api_content_type(env) || body_is_api_content_type(env) + end + + def accepts_api_content_type(env) + is_api_content_type((env['HTTP_ACCEPT'] && env['HTTP_ACCEPT'].downcase) || "") + end + + def body_is_api_content_type(env) + is_api_content_type((env['CONTENT_TYPE'] && env['CONTENT_TYPE'].downcase) || "") + end + + def is_api_content_type(header) + API_CONTENT_TYPES.any?{ |content_type| header.include?(content_type) } + end + + def accept_all(env) + env['HTTP_ACCEPT'] == "*/*" end - def accepts_html?(env) - (env['HTTP_ACCEPT'] || '').include?("text/html") + def is_web_extension(env) + env['PATH_INFO'].end_with?(*WEB_EXTENSIONS) end end end diff --git a/spec/lib/rack/pact_broker/ui_request_filter_spec.rb b/spec/lib/rack/pact_broker/ui_request_filter_spec.rb index 25df8f5cc..7e1c3ab18 100644 --- a/spec/lib/rack/pact_broker/ui_request_filter_spec.rb +++ b/spec/lib/rack/pact_broker/ui_request_filter_spec.rb @@ -21,8 +21,9 @@ module PactBroker end end - context "when the request is for a file" do + context "when the request is for a web file with an Accept header of */*" do let(:path) { "/blah/foo.css" } + let(:accept) { "*/*" } it "forwards the request to the target app" do expect(target_app).to receive(:call) @@ -30,8 +31,16 @@ module PactBroker end end - context "when the request is not for a file, and the Accept header does not include text/html" do - let(:accept) { "application/hal+json, */*" } + context "when the request is for a content type served by the API (HAL browser request)" do + let(:accept) { "application/hal+json, application/json, */*; q=0.01" } + + it "returns a 404" do + expect(subject.status).to eq 404 + end + end + + context "when the request is for an API resource and the Accept headers is */* (default Accept header from curl request)" do + let(:accept) { "*/*" } it "returns a 404" do expect(subject.status).to eq 404