From 47b304d9a810757842c15012f244318d83475088 Mon Sep 17 00:00:00 2001 From: Richard Hatherall Date: Tue, 3 Mar 2026 12:41:26 +0000 Subject: [PATCH] fix: Raise Error on non-2xx responses Replace silent nil returns in Client#get_token and Client#get_user_info with CognitoIdp::Error exceptions that expose error, error_description, and http_status from Cognito's structured error body. BREAKING CHANGE: Methods now raise CognitoIdp::Error instead of returning nil on error responses. --- lib/cognito_idp.rb | 1 + lib/cognito_idp/client.rb | 23 ++++- lib/cognito_idp/error.rb | 19 +++- spec/cognito_idp/client_spec.rb | 149 ++++++++++++++++++++++++++++++-- spec/cognito_idp/error_spec.rb | 29 +++++++ 5 files changed, 213 insertions(+), 8 deletions(-) create mode 100644 spec/cognito_idp/error_spec.rb diff --git a/lib/cognito_idp.rb b/lib/cognito_idp.rb index 8d4f61a..33faf18 100644 --- a/lib/cognito_idp.rb +++ b/lib/cognito_idp.rb @@ -5,6 +5,7 @@ module CognitoIdp autoload :AuthorizationUri, "cognito_idp/authorization_uri" autoload :Client, "cognito_idp/client" + autoload :Error, "cognito_idp/error" autoload :LogoutUri, "cognito_idp/logout_uri" autoload :Token, "cognito_idp/token" autoload :UserInfo, "cognito_idp/user_info" diff --git a/lib/cognito_idp/client.rb b/lib/cognito_idp/client.rb index 5906e44..e27ae25 100644 --- a/lib/cognito_idp/client.rb +++ b/lib/cognito_idp/client.rb @@ -35,7 +35,7 @@ def get_token(grant_type:, **options) scope: options[:scope] }.compact response = connection.post("/oauth2/token", params, basic_authorization_headers) - return unless response.success? + handle_error_response(response) token = Token.new(response.body) yield(token) if block_given? @@ -50,7 +50,7 @@ def get_user_info(token) token end response = connection.post("/oauth2/userInfo", nil, {"Authorization" => "Bearer #{access_token}"}) - return unless response.success? + handle_error_response(response) user_info = UserInfo.new(response.body) yield(user_info) if block_given? @@ -76,6 +76,25 @@ def connection end end + def handle_error_response(response) + return if response.success? + + body = response.body + if body.is_a?(Hash) && body["error"] + raise Error.new( + error: body["error"], + error_description: body["error_description"], + http_status: response.status + ) + else + raise Error.new( + error: "http_error", + error_description: "the server responded with status #{response.status}", + http_status: response.status + ) + end + end + def basic_authorization_headers return if client_secret.nil? diff --git a/lib/cognito_idp/error.rb b/lib/cognito_idp/error.rb index 2d18899..7ac06d4 100644 --- a/lib/cognito_idp/error.rb +++ b/lib/cognito_idp/error.rb @@ -1,5 +1,22 @@ # frozen_string_literal: true module CognitoIdp - class Error < StandardError; end + class Error < StandardError + attr_reader :error, :error_description, :http_status + + def initialize(error:, error_description: nil, http_status: nil) + @error = error + @error_description = error_description + @http_status = http_status + super(build_message) + end + + private + + def build_message + return error if error_description.nil? + + "#{error}: #{error_description}" + end + end end diff --git a/spec/cognito_idp/client_spec.rb b/spec/cognito_idp/client_spec.rb index 140f51a..f2474e6 100644 --- a/spec/cognito_idp/client_spec.rb +++ b/spec/cognito_idp/client_spec.rb @@ -86,6 +86,7 @@ Faraday::Adapter::Test::Stubs.new do |stub| stub.post("https://auth.example.com/oauth2/token") do |env| fail "Authorization is present.#{env.request_headers}" if env.request_headers.key?("Authorization") + [200, {"Content-Type" => "application/json"}, response_payload.to_json] end end end @@ -103,6 +104,7 @@ id_and_secret = "#{client_id}:#{client_secret}" basic_auth = "Basic #{Base64.urlsafe_encode64(id_and_secret)}" fail "Basic Authorization is missing." unless env.request_headers["Authorization"] == basic_auth + [200, {"Content-Type" => "application/json"}, response_payload.to_json] end end end @@ -129,7 +131,66 @@ end let(:error) { "invalid_request" } - it { is_expected.to be_nil } + it "raises a CognitoIdp::Error" do + expect { token }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_request") + expect(e.http_status).to eq(400) + end + end + end + + context "when response is an error with error_description" do + let(:stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("https://auth.example.com/oauth2/token") do |env| + [400, {"Content-Type" => "application/json"}, response_payload.to_json] + end + end + end + let(:response_payload) do + {error: "invalid_grant", error_description: "Authorization code has expired"} + end + + it "raises a CognitoIdp::Error with error_description" do + expect { token }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_grant") + expect(e.error_description).to eq("Authorization code has expired") + expect(e.message).to eq("invalid_grant: Authorization code has expired") + expect(e.http_status).to eq(400) + end + end + end + + context "when response is a server error" do + let(:stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("https://auth.example.com/oauth2/token") do |env| + [500, {"Content-Type" => "text/plain"}, "Internal Server Error"] + end + end + end + + it "raises a CognitoIdp::Error with http_error" do + expect { token }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("http_error") + expect(e.error_description).to eq("the server responded with status 500") + expect(e.http_status).to eq(500) + end + end + end + + context "when response is an error it does not yield" do + let(:stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("https://auth.example.com/oauth2/token") do |env| + [400, {"Content-Type" => "application/json"}, {error: "invalid_request"}.to_json] + end + end + end + + it "does not yield the block" do + expect { |b| client.get_token(grant_type: grant_type, code: code, redirect_uri: redirect_uri, &b) }.to raise_error(CognitoIdp::Error) + end end end @@ -192,7 +253,12 @@ end let(:error) { "invalid_request" } - it { is_expected.to be_nil } + it "raises a CognitoIdp::Error" do + expect { token }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_request") + expect(e.http_status).to eq(400) + end + end end end @@ -243,6 +309,7 @@ Faraday::Adapter::Test::Stubs.new do |stub| stub.post("https://auth.example.com/oauth2/token") do |env| fail "Authorization is present.#{env.request_headers}" if env.request_headers.key?("Authorization") + [200, {"Content-Type" => "application/json"}, response_payload.to_json] end end end @@ -260,6 +327,7 @@ id_and_secret = "#{client_id}:#{client_secret}" basic_auth = "Basic #{Base64.urlsafe_encode64(id_and_secret)}" fail "Basic Authorization is missing." unless env.request_headers["Authorization"] == basic_auth + [200, {"Content-Type" => "application/json"}, response_payload.to_json] end end end @@ -286,7 +354,12 @@ end let(:error) { "invalid_request" } - it { is_expected.to be_nil } + it "raises a CognitoIdp::Error" do + expect { token }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_request") + expect(e.http_status).to eq(400) + end + end end end @@ -349,7 +422,12 @@ end let(:error) { "invalid_request" } - it { is_expected.to be_nil } + it "raises a CognitoIdp::Error" do + expect { token }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_request") + expect(e.http_status).to eq(400) + end + end end end end @@ -453,7 +531,68 @@ end let(:error) { "invalid_request" } - it { is_expected.to be_nil } + it "raises a CognitoIdp::Error" do + expect { user_info }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_request") + expect(e.http_status).to eq(400) + end + end + end + + context "when response is an unauthorized error" do + let(:token) { "ACCESS_TOKEN" } + let(:access_token) { token } + let(:stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("https://auth.example.com/oauth2/userInfo") do |env| + [401, {"Content-Type" => "application/json"}, {error: "invalid_token", error_description: "Access token is expired"}.to_json] + end + end + end + + it "raises a CognitoIdp::Error with error_description" do + expect { user_info }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("invalid_token") + expect(e.error_description).to eq("Access token is expired") + expect(e.http_status).to eq(401) + end + end + end + + context "when response is a server error" do + let(:token) { "ACCESS_TOKEN" } + let(:access_token) { token } + let(:stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("https://auth.example.com/oauth2/userInfo") do |env| + [500, {"Content-Type" => "text/plain"}, "Internal Server Error"] + end + end + end + + it "raises a CognitoIdp::Error with http_error" do + expect { user_info }.to raise_error(CognitoIdp::Error) do |e| + expect(e.error).to eq("http_error") + expect(e.error_description).to eq("the server responded with status 500") + expect(e.http_status).to eq(500) + end + end + end + + context "when response is an error it does not yield" do + let(:token) { "ACCESS_TOKEN" } + let(:access_token) { token } + let(:stubs) do + Faraday::Adapter::Test::Stubs.new do |stub| + stub.post("https://auth.example.com/oauth2/userInfo") do |env| + [400, {"Content-Type" => "application/json"}, {error: "invalid_request"}.to_json] + end + end + end + + it "does not yield the block" do + expect { |b| client.get_user_info(access_token, &b) }.to raise_error(CognitoIdp::Error) + end end end diff --git a/spec/cognito_idp/error_spec.rb b/spec/cognito_idp/error_spec.rb new file mode 100644 index 0000000..b576e02 --- /dev/null +++ b/spec/cognito_idp/error_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.describe CognitoIdp::Error do + it "inherits from StandardError" do + expect(described_class).to be < StandardError + end + + describe "#message" do + context "when only error is provided" do + subject(:error) { described_class.new(error: "invalid_request") } + + it { expect(error.message).to eq("invalid_request") } + end + + context "when error and error_description are provided" do + subject(:error) { described_class.new(error: "invalid_grant", error_description: "Authorization code has expired") } + + it { expect(error.message).to eq("invalid_grant: Authorization code has expired") } + end + end + + describe "attribute readers" do + subject(:error) { described_class.new(error: "invalid_grant", error_description: "Authorization code has expired", http_status: 400) } + + it { expect(error.error).to eq("invalid_grant") } + it { expect(error.error_description).to eq("Authorization code has expired") } + it { expect(error.http_status).to eq(400) } + end +end