From 439f0cac642ea9d0c39abe67e14fae1d5961b07c Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 23 Aug 2024 14:42:28 +1000
Subject: [PATCH] Raise errors on DFC requests

The simplified API was only returning the response body, not allowing us
to inspect if an error occurred. Since an error should be an exception
when communicating with a standardised protocol, we raise an error and
keep our simple API.
---
 engines/dfc_provider/app/services/dfc_request.rb    | 12 +++++++++---
 .../dfc_provider/spec/services/dfc_request_spec.rb  | 13 ++++++++-----
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/engines/dfc_provider/app/services/dfc_request.rb b/engines/dfc_provider/app/services/dfc_request.rb
index 949ec23bad..a24b5d95f3 100644
--- a/engines/dfc_provider/app/services/dfc_request.rb
+++ b/engines/dfc_provider/app/services/dfc_request.rb
@@ -14,9 +14,12 @@ class DfcRequest
   end
 
   def call(url, data = nil)
-    response = request(url, data)
+    begin
+      response = request(url, data)
+    rescue Faraday::UnauthorizedError, Faraday::ForbiddenError
+      raise unless token_stale?
 
-    if response.status >= 400 && token_stale?
+      # If access was denied and our token is stale then refresh and retry:
       refresh_access_token!
       response = request(url, data)
     end
@@ -47,7 +50,10 @@ class DfcRequest
         'Content-Type' => 'application/json',
         'Authorization' => "Bearer #{@user.oidc_account.token}",
       }
-    )
+    ) do |f|
+      # Configure Faraday to raise errors on status 4xx and 5xx responses.
+      f.response :raise_error
+    end
   end
 
   def only_public_connections(&)
diff --git a/engines/dfc_provider/spec/services/dfc_request_spec.rb b/engines/dfc_provider/spec/services/dfc_request_spec.rb
index 0de024b670..0e6fbfb8d3 100644
--- a/engines/dfc_provider/spec/services/dfc_request_spec.rb
+++ b/engines/dfc_provider/spec/services/dfc_request_spec.rb
@@ -40,11 +40,13 @@ RSpec.describe DfcRequest do
 
     expect {
       api.call("http://example.net/api")
-    }.to change {
-      account.token
-    }.and change {
-      account.refresh_token
     }
+      .to raise_error(Faraday::UnauthorizedError)
+      .and change {
+             account.token
+           }.and change {
+                   account.refresh_token
+                 }
   end
 
   it "doesn't try to refresh the token when it's still fresh" do
@@ -53,7 +55,8 @@ RSpec.describe DfcRequest do
 
     user.oidc_account.updated_at = 1.minute.ago
 
-    expect(api.call("http://example.net/api")).to eq ""
+    expect { api.call("http://example.net/api") }
+      .to raise_error(Faraday::UnauthorizedError)
 
     # Trying to reach the OIDC server via network request to refresh the token
     # would raise errors because we didn't setup Webmock or VCR.