Consistently return not found for scoped auth

app/controllers/organisations_controller.rb

@@ -22,7 +22,7 @@ class OrganisationsController < ApplicationController
 private
 
   def authenticate_scope!
-    head :unauthorized if current_user.organisation != @organisation
+    head :not_found if current_user.organisation != @organisation
   end
 
   def find_resource

app/controllers/users_controller.rb

@@ -56,6 +56,6 @@ private
   end
 
   def authenticate_scope!
-    head :unauthorized if current_user != @user
+    head :not_found if current_user != @user
   end
 end

spec/requests/organisations_controller_spec.rb

@@ -5,161 +5,175 @@ RSpec.describe OrganisationsController, type: :request do
   let(:unauthorised_organisation) { FactoryBot.create(:organisation) }
   let(:headers) { { "Accept" => "text/html" } }
   let(:page) { Capybara::Node::Simple.new(response.body) }
+  let(:user) { FactoryBot.create(:user, :data_coordinator) }
 
-  describe "#show" do
+  context "a not signed in user" do
-    let(:user) { FactoryBot.create(:user, :data_coordinator) }
+    it "does not let you see organisation details" do
+      get "/organisations/#{organisation.id}", headers: headers, params: {}
+      expect(response).to redirect_to("/users/sign-in")
 
-    context "organisation that the user belongs to" do
+      get "/organisations/#{organisation.id}/details", headers: headers, params: {}
-      before do
+      expect(response).to redirect_to("/users/sign-in")
-        sign_in user
-        get "/organisations/#{organisation.id}", headers: headers, params: {}
-      end
-
-      it "redirects to details" do
-        expect(response).to have_http_status(:redirect)
-      end
     end
 
-    context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+    it "does not let you see organisation users" do
-      before do
+      get "/organisations/#{organisation.id}/users", headers: headers, params: {}
-        sign_in user
+      expect(response).to redirect_to("/users/sign-in")
-        get "/organisations/#{unauthorised_organisation.id}", headers: headers, params: {}
-      end
-
-      it "returns unauthorised from org route" do
-        expect(response).to have_http_status(:unauthorized)
-      end
     end
   end
 
-  context "As a data coordinator user" do
+  context "a signed in user" do
-    let(:user) { FactoryBot.create(:user, :data_coordinator) }
+    describe "#show" do
-
-    context "details tab" do
       context "organisation that the user belongs to" do
         before do
           sign_in user
-          get "/organisations/#{organisation.id}/details", headers: headers, params: {}
+          get "/organisations/#{organisation.id}", headers: headers, params: {}
-        end
-
-        it "shows the tab navigation" do
-          expected_html = "<nav class=\"app-tab-navigation\""
-          expect(response.body).to include(expected_html)
-        end
-
-        it "shows a summary list of org details" do
-          expected_html = "<dl class=\"govuk-summary-list\""
-          expect(response.body).to include(expected_html)
-          expect(response.body).to include(organisation.name)
         end
 
-        it "has a hidden header title" do
+        it "redirects to details" do
-          expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
+          expect(response).to have_http_status(:redirect)
-          expect(response.body).to include(expected_html)
         end
       end
 
       context "organisation that are not in scope for the user, i.e. that they do not belong to" do
         before do
           sign_in user
-          get "/organisations/#{unauthorised_organisation.id}/details", headers: headers, params: {}
+          get "/organisations/#{unauthorised_organisation.id}", headers: headers, params: {}
         end
 
-        it "returns unauthorised from org details route" do
+        it "returns not found 404 from org route" do
-          expect(response).to have_http_status(:unauthorized)
+          expect(response).to have_http_status(:not_found)
         end
       end
     end
 
-    context "users tab" do
+    context "As a data coordinator user" do
-      context "organisation that the user belongs to" do
+      context "details tab" do
-        before do
+        context "organisation that the user belongs to" do
-          sign_in user
+          before do
-          get "/organisations/#{organisation.id}/users", headers: headers, params: {}
+            sign_in user
+            get "/organisations/#{organisation.id}/details", headers: headers, params: {}
+          end
+
+          it "shows the tab navigation" do
+            expected_html = "<nav class=\"app-tab-navigation\""
+            expect(response.body).to include(expected_html)
+          end
+
+          it "shows a summary list of org details" do
+            expected_html = "<dl class=\"govuk-summary-list\""
+            expect(response.body).to include(expected_html)
+            expect(response.body).to include(organisation.name)
+          end
+
+          it "has a hidden header title" do
+            expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
+            expect(response.body).to include(expected_html)
+          end
+        end
+
+        context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+          before do
+            sign_in user
+            get "/organisations/#{unauthorised_organisation.id}/details", headers: headers, params: {}
+          end
+
+          it "returns not found 404 from org details route" do
+            expect(response).to have_http_status(:not_found)
+          end
         end
+      end
 
-        it "shows the tab navigation" do
+      context "users tab" do
-          expected_html = "<nav class=\"app-tab-navigation\""
+        context "organisation that the user belongs to" do
-          expect(response.body).to include(expected_html)
+          before do
-        end
+            sign_in user
+            get "/organisations/#{organisation.id}/users", headers: headers, params: {}
+          end
 
-        it "shows a new user button" do
+          it "shows the tab navigation" do
-          expect(page).to have_link("Invite user")
+            expected_html = "<nav class=\"app-tab-navigation\""
-        end
+            expect(response.body).to include(expected_html)
+          end
 
-        it "shows a table of users" do
+          it "shows a new user button" do
-          expected_html = "<table class=\"govuk-table\""
+            expect(page).to have_link("Invite user")
-          expect(response.body).to include(expected_html)
+          end
-          expect(response.body).to include(user.email)
-        end
 
-        it "has a hidden header title" do
+          it "shows a table of users" do
-          expected_html = "<h2 class=\"govuk-visually-hidden\">  Users"
+            expected_html = "<table class=\"govuk-table\""
-          expect(response.body).to include(expected_html)
+            expect(response.body).to include(expected_html)
-        end
+            expect(response.body).to include(user.email)
-      end
+          end
 
-      context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+          it "has a hidden header title" do
-        before do
+            expected_html = "<h2 class=\"govuk-visually-hidden\">  Users"
-          sign_in user
+            expect(response.body).to include(expected_html)
-          get "/organisations/#{unauthorised_organisation.id}/users", headers: headers, params: {}
+          end
         end
 
-        it "returns unauthorised from users page" do
+        context "organisation that are not in scope for the user, i.e. that they do not belong to" do
-          expect(response).to have_http_status(:unauthorized)
+          before do
+            sign_in user
+            get "/organisations/#{unauthorised_organisation.id}/users", headers: headers, params: {}
+          end
+
+          it "returns not found 404 from users page" do
+            expect(response).to have_http_status(:not_found)
+          end
         end
       end
     end
-  end
 
-  context "As a data provider user" do
+    context "As a data provider user" do
-    let(:user) { FactoryBot.create(:user) }
+      let(:user) { FactoryBot.create(:user) }
 
-    context "details tab" do
+      context "details tab" do
-      context "organisation that the user belongs to" do
+        context "organisation that the user belongs to" do
-        before do
+          before do
-          sign_in user
+            sign_in user
-          get "/organisations/#{organisation.id}/details", headers: headers, params: {}
+            get "/organisations/#{organisation.id}/details", headers: headers, params: {}
-        end
+          end
 
-        it "shows the tab navigation" do
+          it "shows the tab navigation" do
-          expected_html = "<nav class=\"app-tab-navigation\""
+            expected_html = "<nav class=\"app-tab-navigation\""
-          expect(response.body).to include(expected_html)
+            expect(response.body).to include(expected_html)
-        end
+          end
+
+          it "shows a summary list of org details" do
+            expected_html = "<dl class=\"govuk-summary-list\""
+            expect(response.body).to include(expected_html)
+            expect(response.body).to include(organisation.name)
+          end
 
-        it "shows a summary list of org details" do
+          it "has a hidden header title" do
-          expected_html = "<dl class=\"govuk-summary-list\""
+            expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
-          expect(response.body).to include(expected_html)
+            expect(response.body).to include(expected_html)
-          expect(response.body).to include(organisation.name)
+          end
         end
 
-        it "has a hidden header title" do
+        context "organisation that are not in scope for the user, i.e. that they do not belong to" do
-          expected_html = "<h2 class=\"govuk-visually-hidden\">  Details"
+          before do
-          expect(response.body).to include(expected_html)
+            sign_in user
+            get "/organisations/#{unauthorised_organisation.id}/details", headers: headers, params: {}
+          end
+
+          it "returns not found 404" do
+            expect(response).to have_http_status(:not_found)
+          end
         end
       end
 
-      context "organisation that are not in scope for the user, i.e. that they do not belong to" do
+      context "users tab" do
         before do
           sign_in user
-          get "/organisations/#{unauthorised_organisation.id}/details", headers: headers, params: {}
+          get "/organisations/#{organisation.id}/users", headers: headers, params: {}
         end
 
-        it "returns unauthorised" do
+        it "should return unauthorized 401" do
           expect(response).to have_http_status(:unauthorized)
         end
       end
     end
-
-    context "users tab" do
-      before do
-        sign_in user
-        get "/organisations/#{organisation.id}/users", headers: headers, params: {}
-      end
-
-      it "should return unauthorised 401" do
-        expect(response).to have_http_status(:unauthorized)
-      end
-    end
   end
 end

spec/requests/user_controller_spec.rb

@@ -6,6 +6,30 @@ RSpec.describe UsersController, type: :request do
   let(:unauthorised_user) { FactoryBot.create(:user) }
   let(:headers) { { "Accept" => "text/html" } }
   let(:page) { Capybara::Node::Simple.new(response.body) }
+  let(:new_value) { "new test name" }
+  let(:params) { { id: user.id, user: { name: new_value } } }
+
+  context "a not signed in user" do
+    it "does not let you see user details" do
+      get "/users/#{user.id}", headers: headers, params: {}
+      expect(response).to redirect_to("/users/sign-in")
+    end
+
+    it "does not let you edit user details" do
+      get "/users/#{user.id}/edit", headers: headers, params: {}
+      expect(response).to redirect_to("/users/sign-in")
+    end
+
+    it "does not let you edit user passwords" do
+      get "/users/#{user.id}/password/edit", headers: headers, params: {}
+      expect(response).to redirect_to("/users/sign-in")
+    end
+
+    it "does not let you update user details" do
+      patch "/case-logs/#{user.id}", params: {}
+      expect(response).to redirect_to("/users/sign-in")
+    end
+  end
 
   describe "#show" do
     context "current user is user" do
@@ -25,8 +49,8 @@ RSpec.describe UsersController, type: :request do
         get "/users/#{unauthorised_user.id}", headers: headers, params: {}
       end
 
-      it "returns unauthorised 401" do
+      it "returns not found 404" do
-        expect(response).to have_http_status(:unauthorized)
+        expect(response).to have_http_status(:not_found)
       end
     end
   end
@@ -49,8 +73,8 @@ RSpec.describe UsersController, type: :request do
         get "/users/#{unauthorised_user.id}/edit", headers: headers, params: {}
       end
 
-      it "returns unauthorised 401" do
+      it "returns not found 404" do
-        expect(response).to have_http_status(:unauthorized)
+        expect(response).to have_http_status(:not_found)
       end
     end
   end
@@ -73,16 +97,13 @@ RSpec.describe UsersController, type: :request do
         get "/users/#{unauthorised_user.id}/edit", headers: headers, params: {}
       end
 
-      it "returns unauthorised 401" do
+      it "returns not found 404" do
-        expect(response).to have_http_status(:unauthorized)
+        expect(response).to have_http_status(:not_found)
       end
     end
   end
 
   describe "#update" do
-    let(:new_value) { "new test name" }
-    let(:params) { { id: user.id, user: { name: new_value } } }
-
     context "current user is user" do
       before do
         sign_in user
@@ -103,8 +124,8 @@ RSpec.describe UsersController, type: :request do
         patch "/users/#{unauthorised_user.id}", headers: headers, params: params
       end
 
-      it "returns unauthorised 401" do
+      it "returns not found 404" do
-        expect(response).to have_http_status(:unauthorized)
+        expect(response).to have_http_status(:not_found)
       end
     end
   end