From b01020c9592cde16730a64d3cf24c8f1b62d66d4 Mon Sep 17 00:00:00 2001 From: iamibi <8592115+iamibi@users.noreply.github.com> Date: Thu, 17 Sep 2026 11:45:12 -0700 Subject: [PATCH] Add spec coverage for Client::Helper, query_string scheme, and Errors classes Coverage dropped from 88.32% to 69.47% line coverage between v1.1.7 and v1.1.8. lib/oauth/client/ was the weakest real directory, with OAuth::Client::Helper, the query_string OAuth scheme, and the OAuth error classes having no dedicated spec files. --- CHANGELOG.md | 4 + spec/net/http_spec.rb | 47 ++++++++ spec/oauth/client/helper_spec.rb | 184 +++++++++++++++++++++++++++++++ spec/oauth/error_spec.rb | 13 +++ spec/oauth/problem_spec.rb | 34 ++++++ spec/oauth/unauthorized_spec.rb | 36 ++++++ 6 files changed, 318 insertions(+) create mode 100644 spec/oauth/client/helper_spec.rb create mode 100644 spec/oauth/error_spec.rb create mode 100644 spec/oauth/problem_spec.rb create mode 100644 spec/oauth/unauthorized_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 07fb7397..33c3a42e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ Please file a bug if you notice a violation of semantic versioning. ### Added +- Spec coverage for `OAuth::Client::Helper`, the `query_string` OAuth signing scheme in + `lib/oauth/client/net_http.rb`, and the `OAuth::Error`/`OAuth::Unauthorized`/`OAuth::Problem` + classes — none previously had a dedicated spec file. + ### Changed ### Deprecated diff --git a/spec/net/http_spec.rb b/spec/net/http_spec.rb index 22c062d9..47950fea 100644 --- a/spec/net/http_spec.rb +++ b/spec/net/http_spec.rb @@ -38,4 +38,51 @@ expect(request.path).to eq("/test") expect(request["authorization"]).to include("oauth_body_hash=") end + + describe "scheme: query_string" do + it "appends OAuth parameters as the query when the request URI has none" do + request = Net::HTTP::Get.new("/test") + request.oauth!(http, consumer, token, {scheme: "query_string", nonce: nonce, timestamp: timestamp}) + + expect(request["authorization"]).to be_nil + expect(request.path).to start_with("/test?oauth_consumer_key=#{consumer.key}") + expect(request.path).to include("oauth_nonce=#{nonce}") + expect(request.path).to match(/&oauth_signature=[^&]+\z/) + end + + it "appends OAuth parameters to an existing query string with '&'" do + request = Net::HTTP::Get.new("#{uri.path}?key=value") + request.oauth!(http, consumer, token, {scheme: "query_string", nonce: nonce, timestamp: timestamp}) + + expect(request["authorization"]).to be_nil + # existing params are preserved, OAuth params are appended after them + expect(request.path).to start_with("/test?key=value&oauth_consumer_key=#{consumer.key}") + expect(request.path).to match(/&oauth_signature=[^&]+\z/) + end + end + + describe "#signature_base_string uri derivation" do + it "derives an https normalized URI from the http object when no :site option is given" do + https_http = described_class.new(uri.host, uri.port) + https_http.use_ssl = true + request = Net::HTTP::Get.new(uri.path) + + base_string = request.signature_base_string(https_http, consumer, token, {nonce: nonce, timestamp: timestamp}) + + expect(base_string).to include("https%3A%2F%2Fexample.com") + end + + it "derives host/port from :site when :request_endpoint is set" do + request = Net::HTTP::Get.new(uri.path) + + base_string = request.signature_base_string(http, consumer, token, { + request_endpoint: true, + site: "https://api.example.com", + nonce: nonce, + timestamp: timestamp + }) + + expect(base_string).to include("api.example.com") + end + end end diff --git a/spec/oauth/client/helper_spec.rb b/spec/oauth/client/helper_spec.rb new file mode 100644 index 00000000..f0851083 --- /dev/null +++ b/spec/oauth/client/helper_spec.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +require "spec_helper" +require "net/http" +require "oauth/client/helper" + +RSpec.describe OAuth::Client::Helper do + subject(:helper) { described_class.new(request, base_options) } + + let(:consumer) { OAuth::Consumer.new("consumer_key_86cad9", "5888bf0345e5d237") } + let(:token) { OAuth::Token.new("token_411a7f", "3196ffd991c8ebdb") } + let(:uri) { URI.parse("http://example.com/test?key=value") } + let(:request) { Net::HTTP::Get.new("#{uri.path}?key=value") } + let(:nonce) { 225_579_211_881_198_842_005_988_698_334_675_835_446 } + let(:timestamp) { "1199645624" } + let(:base_options) do + { + consumer: consumer, + token: token, + request_uri: uri.to_s, + nonce: nonce, + timestamp: timestamp + } + end + + describe "#initialize" do + it "defaults signature_method to HMAC-SHA1 when not provided" do + expect(helper.options[:signature_method]).to eq("HMAC-SHA1") + end + + it "keeps an explicitly provided signature_method" do + explicit = described_class.new(request, base_options.merge(signature_method: "PLAINTEXT")) + expect(explicit.options[:signature_method]).to eq("PLAINTEXT") + end + end + + describe "#nonce" do + it "memoizes the provided nonce into options" do + expect(helper.nonce).to eq(nonce) + expect(helper.options[:nonce]).to eq(nonce) + end + + it "generates and memoizes a nonce when none is provided" do + fresh = described_class.new(request, base_options.reject { |k, _| k == :nonce }) + generated = fresh.nonce + expect(generated).not_to be_nil + expect(fresh.nonce).to eq(generated) + end + end + + describe "#timestamp" do + it "memoizes the provided timestamp into options" do + expect(helper.timestamp).to eq(timestamp) + expect(helper.options[:timestamp]).to eq(timestamp) + end + + it "generates and memoizes a timestamp when none is provided" do + fresh = described_class.new(request, base_options.reject { |k, _| k == :timestamp }) + generated = fresh.timestamp + expect(generated).not_to be_nil + expect(fresh.timestamp).to eq(generated) + end + end + + describe "#oauth_parameters" do + it "includes the core OAuth parameters" do + params = helper.oauth_parameters + expect(params["oauth_consumer_key"]).to eq("consumer_key_86cad9") + expect(params["oauth_token"]).to eq("token_411a7f") + expect(params["oauth_signature_method"]).to eq("HMAC-SHA1") + expect(params["oauth_nonce"]).to eq(nonce) + expect(params["oauth_timestamp"]).to eq(timestamp) + expect(params["oauth_version"]).to eq("1.0") + end + + it "omits empty-valued parameters by default" do + params = helper.oauth_parameters + expect(params).not_to have_key("oauth_verifier") + expect(params).not_to have_key("oauth_session_handle") + expect(params).not_to have_key("oauth_body_hash") + expect(params).not_to have_key("oauth_callback") + end + + it "keeps every empty-valued parameter when allow_empty_params is true" do + with_flag = described_class.new(request, base_options.merge(allow_empty_params: true)) + params = with_flag.oauth_parameters + expect(params).to have_key("oauth_verifier") + expect(params).to have_key("oauth_session_handle") + end + + it "keeps only the named parameter when allow_empty_params is an array" do + with_array = described_class.new(request, base_options.merge(allow_empty_params: ["oauth_verifier"])) + params = with_array.oauth_parameters + expect(params).to have_key("oauth_verifier") + expect(params).not_to have_key("oauth_session_handle") + end + + it "keeps only the named parameter when allow_empty_params is a single string" do + with_string = described_class.new(request, base_options.merge(allow_empty_params: "oauth_verifier")) + params = with_string.oauth_parameters + expect(params).to have_key("oauth_verifier") + expect(params).not_to have_key("oauth_session_handle") + end + + it "omits every empty-valued parameter when allow_empty_params is explicitly false" do + with_false = described_class.new(request, base_options.merge(allow_empty_params: false)) + params = with_false.oauth_parameters + expect(params).not_to have_key("oauth_verifier") + end + end + + describe "#token_request?" do + it "is false by default" do + expect(helper.token_request?).to be(false) + end + + it "is true only when options[:token_request] is exactly true" do + token_request = described_class.new(request, base_options.merge(token_request: true)) + expect(token_request.token_request?).to be(true) + + truthy_but_not_true = described_class.new(request, base_options.merge(token_request: "yes")) + expect(truthy_but_not_true.token_request?).to be(false) + end + end + + describe "#hash_body" do + it "computes and memoizes a body hash into options[:body_hash]" do + expect(helper.options[:body_hash]).to be_nil + hash = helper.hash_body + expect(hash).not_to be_nil + expect(helper.options[:body_hash]).to eq(hash) + end + end + + describe "#amend_user_agent_header" do + it "sets a fresh User-Agent header when none is present" do + headers = {} + helper.amend_user_agent_header(headers) + expect(headers["User-Agent"]).to eq("OAuth gem v#{OAuth::Version::VERSION}") + end + + it "treats a bare Ruby default User-Agent the same as absent" do + headers = {"User-Agent" => "Ruby"} + helper.amend_user_agent_header(headers) + expect(headers["User-Agent"]).to eq("OAuth gem v#{OAuth::Version::VERSION}") + end + + it "appends to an existing, non-default User-Agent header instead of replacing it" do + headers = {"User-Agent" => "MyApp/1.0"} + helper.amend_user_agent_header(headers) + expect(headers["User-Agent"]).to eq("MyApp/1.0 (OAuth gem v#{OAuth::Version::VERSION})") + end + end + + describe "#header" do + it "produces a well-formed OAuth Authorization header with sorted parameters" do + header = helper.header + expect(header).to start_with("OAuth ") + expect(header).to include("oauth_signature=") + expect(header).to include("oauth_consumer_key=\"consumer_key_86cad9\"") + keys = header.sub(/^OAuth /, "").split(", ").map { |pair| pair.split("=").first } + expect(keys).to eq(keys.sort) + end + + it "prefixes the header with realm when options[:realm] is set" do + with_realm = described_class.new(request, base_options.merge(realm: "http://example.com")) + expect(with_realm.header).to start_with("OAuth realm=\"http://example.com\", ") + end + end + + describe "#parameters" do + it "delegates to the request proxy's parameters" do + expect(helper.parameters).to eq(OAuth::RequestProxy.proxy(request).parameters) + end + end + + describe "#parameters_with_oauth" do + it "merges the OAuth parameters with the request's own parameters" do + merged = helper.parameters_with_oauth + expect(merged["oauth_consumer_key"]).to eq("consumer_key_86cad9") + expect(merged["key"]).to eq(helper.parameters["key"]) + end + end +end diff --git a/spec/oauth/error_spec.rb b/spec/oauth/error_spec.rb new file mode 100644 index 00000000..29c4e99a --- /dev/null +++ b/spec/oauth/error_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe OAuth::Error do + it "is a StandardError" do + expect(described_class.ancestors).to include(StandardError) + end + + it "can be raised and rescued with a custom message" do + expect { raise described_class, "went wrong" }.to raise_error(described_class, "went wrong") + end +end diff --git a/spec/oauth/problem_spec.rb b/spec/oauth/problem_spec.rb new file mode 100644 index 00000000..17ebc425 --- /dev/null +++ b/spec/oauth/problem_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe OAuth::Problem do + it "is an OAuth::Unauthorized" do + expect(described_class.ancestors).to include(OAuth::Unauthorized) + end + + describe "#initialize" do + it "stores problem, request, and params" do + request = double("request") + problem = described_class.new("token_expired", request, {"oauth_problem" => "token_expired"}) + + expect(problem.problem).to eq("token_expired") + expect(problem.request).to eq(request) + expect(problem.params).to eq({"oauth_problem" => "token_expired"}) + end + + it "defaults request to nil and params to an empty hash" do + problem = described_class.new("token_expired") + + expect(problem.request).to be_nil + expect(problem.params).to eq({}) + end + end + + describe "#to_s" do + it "returns the problem string, overriding Unauthorized#to_s" do + problem = described_class.new("token_expired") + expect(problem.to_s).to eq("token_expired") + end + end +end diff --git a/spec/oauth/unauthorized_spec.rb b/spec/oauth/unauthorized_spec.rb new file mode 100644 index 00000000..d1012d42 --- /dev/null +++ b/spec/oauth/unauthorized_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe OAuth::Unauthorized do + it "is an OAuth::Error" do + expect(described_class.ancestors).to include(OAuth::Error) + end + + describe "#request" do + it "defaults to nil" do + expect(described_class.new.request).to be_nil + end + + it "returns the request it was constructed with" do + request = double("request") + expect(described_class.new(request).request).to eq(request) + end + end + + describe "#to_s" do + it "returns a generic message when no request is present" do + expect(described_class.new.to_s).to eq("401 Unauthorized") + end + + it "returns the request's code and message when a request is present" do + request = double("request", code: "401", message: "Unauthorized") + expect(described_class.new(request).to_s).to eq("401 Unauthorized") + end + + it "reflects whatever code/message the underlying request reports" do + request = double("request", code: "419", message: "Session Expired") + expect(described_class.new(request).to_s).to eq("419 Session Expired") + end + end +end