From 61d0df14472db34e647671a531ce69cddc03e486 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 12 Feb 2025 16:07:44 +1100
Subject: [PATCH] Add spec

Oh, and a transaction block. Because the controller after hooks tried to update the DB which resulted in
  PG::InFailedSqlTransaction: ERROR:  current transaction is aborted, commands ignored until end of transaction block

Even for a small rescue statement, it's worth adding a spec. You never know what might not be working!
---
 app/controllers/omniauth_callbacks_controller.rb  |  4 +++-
 .../omniauth_callbacks_controller_spec.rb         | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index d55c5600e21..31f9a29ac38 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -2,7 +2,9 @@
 
 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   def openid_connect
-    OidcAccount.link(spree_current_user, request.env["omniauth.auth"])
+    ActiveRecord::Base.transaction do
+      OidcAccount.link(spree_current_user, request.env["omniauth.auth"])
+    end
 
     redirect_to admin_oidc_settings_path
   rescue ActiveRecord::RecordNotUnique
diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb
index aef5ef69d64..8f28c26bcb6 100644
--- a/spec/requests/omniauth_callbacks_controller_spec.rb
+++ b/spec/requests/omniauth_callbacks_controller_spec.rb
@@ -35,6 +35,20 @@ def request!
       expect(account.uid).to eq "ofn@example.com"
       expect(response.status).to eq(302)
     end
+
+    context 'when OIDC account already linked with a different user' do
+      before do
+        other_user = create(:user, email: "ofn@elsewhere.com")
+        OidcAccount.create! user_id: other_user.id, uid: "ofn@example.com"
+      end
+
+      it 'fails with error message' do
+        expect { request! }.not_to change { OidcAccount.count }
+
+        expect(response.status).to eq(302)
+        expect(flash[:error]).to match "is already associated with another account"
+      end
+    end
   end
 
   context 'when the omniauth openid_connect is mocked with an error' do
@@ -46,6 +60,7 @@ def request!
       expect { request! }.not_to change { OidcAccount.count }
 
       expect(response.status).to eq(302)
+      expect(flash[:error]).to match "Could not sign in"
     end
   end
 end