From acec73d21ac36c9e57858f342fe89db56d98b209 Mon Sep 17 00:00:00 2001
From: Alex Avlonitis <alex.avlonitis@digital.cabinet-office.gov.uk>
Date: Mon, 22 Apr 2024 15:42:40 +0100
Subject: [PATCH] Re-use authorisation logic in the controller_methods class

---
 lib/gds-sso/controller_methods.rb          | 33 +-----------
 spec/controller/controller_methods_spec.rb | 61 +++++-----------------
 2 files changed, 15 insertions(+), 79 deletions(-)

diff --git a/lib/gds-sso/controller_methods.rb b/lib/gds-sso/controller_methods.rb
index 85dfe47..37bde23 100644
--- a/lib/gds-sso/controller_methods.rb
+++ b/lib/gds-sso/controller_methods.rb
@@ -32,22 +32,7 @@ def authorise_user!(permissions)
         # Otherwise current_user might be nil, and we'd error out
         authenticate_user!
 
-        case permissions
-        when String
-          unless current_user.has_permission?(permissions)
-            raise PermissionDeniedException, "Sorry, you don't seem to have the #{permissions} permission for this app."
-          end
-        when Hash
-          raise ArgumentError, "Must be either `any_of` or `all_of`" unless permissions.keys.size == 1
-
-          if permissions[:any_of]
-            authorise_user_with_at_least_one_of_permissions!(permissions[:any_of])
-          elsif permissions[:all_of]
-            authorise_user_with_all_permissions!(permissions[:all_of])
-          else
-            raise ArgumentError, "Must be either `any_of` or `all_of`"
-          end
-        end
+        GDS::SSO::AuthoriseUser.call(current_user, permissions)
       end
 
       def authenticate_user!
@@ -73,22 +58,6 @@ def logout
       def warden
         request.env["warden"]
       end
-
-    private
-
-      def authorise_user_with_at_least_one_of_permissions!(permissions)
-        if permissions.none? { |permission| current_user.has_permission?(permission) }
-          raise PermissionDeniedException,
-                "Sorry, you don't seem to have any of the permissions: #{permissions.to_sentence} for this app."
-        end
-      end
-
-      def authorise_user_with_all_permissions!(permissions)
-        unless permissions.all? { |permission| current_user.has_permission?(permission) }
-          raise PermissionDeniedException,
-                "Sorry, you don't seem to have all of the permissions: #{permissions.to_sentence} for this app."
-        end
-      end
     end
   end
 end
diff --git a/spec/controller/controller_methods_spec.rb b/spec/controller/controller_methods_spec.rb
index 67d63a2..9100fd5 100644
--- a/spec/controller/controller_methods_spec.rb
+++ b/spec/controller/controller_methods_spec.rb
@@ -1,57 +1,24 @@
 require "spec_helper"
 
-RSpec.describe GDS::SSO::ControllerMethods, "#authorise_user!" do
-  let(:current_user) { double }
-  let(:expected_error) { GDS::SSO::ControllerMethods::PermissionDeniedException }
+RSpec.describe GDS::SSO::ControllerMethods do
+  describe "#authorise_user!" do
+    let(:current_user) { double }
+    let(:expected_error) { GDS::SSO::PermissionDeniedError }
 
-  context "with a single string permission argument" do
-    it "permits users with the required permission" do
-      allow(current_user).to receive(:has_permission?).with("good").and_return(true)
+    context "when the user is authorised" do
+      it "does not raise an error" do
+        allow(current_user).to receive(:has_permission?).with("good").and_return(true)
 
-      expect { ControllerSpy.new(current_user).authorise_user!("good") }.not_to raise_error
+        expect { ControllerSpy.new(current_user).authorise_user!("good") }.not_to raise_error
+      end
     end
 
-    it "does not permit the users without the required permission" do
-      allow(current_user).to receive(:has_permission?).with("good").and_return(false)
+    context "when the user is not authorised" do
+      it "raises an error" do
+        allow(current_user).to receive(:has_permission?).with("bad").and_return(false)
 
-      expect { ControllerSpy.new(current_user).authorise_user!("good") }.to raise_error(expected_error)
-    end
-  end
-
-  context "with the `all_of` option" do
-    it "permits users with all of the required permissions" do
-      allow(current_user).to receive(:has_permission?).with("good").and_return(true)
-      allow(current_user).to receive(:has_permission?).with("bad").and_return(true)
-
-      expect { ControllerSpy.new(current_user).authorise_user!(all_of: %w[good bad]) }.not_to raise_error
-    end
-
-    it "does not permit users without all of the required permissions" do
-      allow(current_user).to receive(:has_permission?).with("good").and_return(false)
-      allow(current_user).to receive(:has_permission?).with("bad").and_return(true)
-
-      expect { ControllerSpy.new(current_user).authorise_user!(all_of: %w[good bad]) }.to raise_error(expected_error)
-    end
-  end
-
-  context "with the `any_of` option" do
-    it "permits users with any of the required permissions" do
-      allow(current_user).to receive(:has_permission?).with("good").and_return(true)
-      allow(current_user).to receive(:has_permission?).with("bad").and_return(false)
-
-      expect { ControllerSpy.new(current_user).authorise_user!(any_of: %w[good bad]) }.not_to raise_error
-    end
-
-    it "does not permit users without any of the required permissions" do
-      allow(current_user).to receive(:has_permission?).and_return(false)
-
-      expect { ControllerSpy.new(current_user).authorise_user!(any_of: %w[good bad]) }.to raise_error(expected_error)
-    end
-  end
-
-  context "with none of `any_of` or `all_of`" do
-    it "raises an `ArgumentError`" do
-      expect { ControllerSpy.new(current_user).authorise_user!(whoops: "bad") }.to raise_error(ArgumentError)
+        expect { ControllerSpy.new(current_user).authorise_user!("bad") }.to raise_error(expected_error)
+      end
     end
   end
 end