Restrict people/org access to admins only

app/controllers/people_controller.rb

@@ -29,8 +29,8 @@ def index
   end
 
   def show
-    @person = Person.find(params[:id]).decorate
     authorize! @person
+    @person = @person.decorate
     track_view(@person)
 
     # Handle paginated sections for Turbo Frame requests

app/policies/organization_policy.rb

@@ -2,11 +2,11 @@ class OrganizationPolicy < ApplicationPolicy
   # See https://actionpolicy.evilmartians.io/#/writing_policies
   #
   def index?
-    authenticated?
+    admin?
   end
 
   def show?
-    admin? || (authenticated? && record.published?)
+    admin?
   end
 
   def show_workshop_logs?

app/policies/person_policy.rb

@@ -2,11 +2,11 @@ class PersonPolicy < ApplicationPolicy
   # See https://actionpolicy.evilmartians.io/#/writing_policies
 
   def index?
-    authenticated?
+    admin?
   end
 
   def show?
-    admin? || owner? || (authenticated? && record.profile_is_searchable?)
+    admin? || owner?
   end
 
   def edit?

spec/policies/organization_policy_spec.rb

@@ -0,0 +1,86 @@
+require "rails_helper"
+
+RSpec.describe OrganizationPolicy, type: :policy do
+  let(:admin_user) { build_stubbed(:user, :admin) }
+  let(:regular_user) { build_stubbed(:user) }
+
+  let(:organization) { build_stubbed(:organization) }
+
+  def policy_for(record: nil, user:)
+    described_class.new(record, user: user)
+  end
+
+  describe "#index?" do
+    context "with admin user" do
+      subject { policy_for(user: admin_user) }
+
+      it { is_expected.to be_allowed_to(:index?) }
+    end
+
+    context "with regular user" do
+      subject { policy_for(user: regular_user) }
+
+      it { is_expected.not_to be_allowed_to(:index?) }
+    end
+
+    context "with no user" do
+      subject { policy_for(user: nil) }
+
+      it { is_expected.not_to be_allowed_to(:index?) }
+    end
+  end
+
+  describe "#show?" do
+    context "with admin user" do
+      subject { policy_for(record: organization, user: admin_user) }
+
+      it { is_expected.to be_allowed_to(:show?) }
+    end
+
+    context "with regular user" do
+      subject { policy_for(record: organization, user: regular_user) }
+
+      it { is_expected.not_to be_allowed_to(:show?) }
+    end
+
+    context "with no user" do
+      subject { policy_for(record: organization, user: nil) }
+
+      it { is_expected.not_to be_allowed_to(:show?) }
+    end
+  end
+
+  describe "#populations_served?" do
+    context "with admin user" do
+      subject { policy_for(record: organization, user: admin_user) }
+
+      it { is_expected.to be_allowed_to(:populations_served?) }
+    end
+
+    context "with regular user" do
+      subject { policy_for(record: organization, user: regular_user) }
+
+      it { is_expected.not_to be_allowed_to(:populations_served?) }
+    end
+  end
+
+  describe "relation_scope" do
+    context "with admin user" do
+      let(:policy) { policy_for(record: Organization, user: admin_user) }
+
+      it "returns all organizations" do
+        scope = policy.apply_scope(Organization.all, type: :active_record_relation)
+        expect(scope).to eq(Organization.all)
+      end
+    end
+
+    context "with regular user" do
+      let(:policy) { policy_for(record: Organization, user: regular_user) }
+
+      it "filters to published organizations" do
+        scope = policy.apply_scope(Organization.all, type: :active_record_relation)
+        expect(scope.to_sql).to eq(Organization.published.to_sql)
+      end
+    end
+  end
+end

spec/policies/person_policy_spec.rb

@@ -23,7 +23,7 @@ def policy_for(record: nil, user:)
     context "with regular user" do
       subject { policy_for(user: regular_user) }
 
-      it { is_expected.to be_allowed_to(:index?) }
+      it { is_expected.not_to be_allowed_to(:index?) }
     end
 
     context "with no user" do
@@ -49,7 +49,7 @@ def policy_for(record: nil, user:)
     context "with regular user and searchable person" do
       subject { policy_for(record: searchable_person, user: regular_user) }
 
-      it { is_expected.to be_allowed_to(:show?) }
+      it { is_expected.not_to be_allowed_to(:show?) }
     end
 
     context "with regular user and non-searchable person" do

spec/requests/organizations_authorization_spec.rb

@@ -0,0 +1,63 @@
+require "rails_helper"
+
+RSpec.describe "Organizations authorization", type: :request do
+  let(:admin) { create(:user, :admin) }
+  let(:regular_user) { create(:user) }
+
+  let!(:organization_status) { create(:organization_status, name: "Active") }
+  let!(:organization) { create(:organization, organization_status: organization_status) }
+
+  describe "GET /organizations" do
+    context "as a visitor" do
+      it "redirects to root" do
+        get organizations_path
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as a regular user" do
+      before { sign_in regular_user }
+
+      it "redirects to root" do
+        get organizations_path
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as an admin" do
+      before { sign_in admin }
+
+      it "renders successfully" do
+        get organizations_path
+        expect(response).to have_http_status(:ok)
+      end
+    end
+  end
+
+  describe "GET /organizations/:id" do
+    context "as a visitor" do
+      it "redirects to root" do
+        get organization_path(organization)
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as a regular user" do
+      before { sign_in regular_user }
+
+      it "redirects to root" do
+        get organization_path(organization)
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as an admin" do
+      before { sign_in admin }
+
+      it "renders successfully" do
+        get organization_path(organization)
+        expect(response).to have_http_status(:ok)
+      end
+    end
+  end
+end

spec/requests/people_authorization_spec.rb

@@ -0,0 +1,66 @@
+require "rails_helper"
+
+RSpec.describe "People authorization", type: :request do
+  let(:admin) { create(:user, :admin) }
+  let(:regular_user) { create(:user, :with_person) }
+  let(:other_person) { create(:person) }
+
+  describe "GET /people" do
+    context "as a visitor" do
+      it "redirects to root" do
+        get people_path
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as a regular user" do
+      before { sign_in regular_user }
+
+      it "redirects to root" do
+        get people_path
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as an admin" do
+      before { sign_in admin }
+
+      it "renders successfully" do
+        get people_path
+        expect(response).to have_http_status(:ok)
+      end
+    end
+  end
+
+  describe "GET /people/:id" do
+    context "as a visitor" do
+      it "redirects to root" do
+        get person_path(other_person)
+        expect(response).to redirect_to(root_path)
+      end
+    end
+
+    context "as a regular user" do
+      before { sign_in regular_user }
+
+      it "redirects to root for another person" do
+        get person_path(other_person)
+        expect(response).to redirect_to(root_path)
+      end
+
+      it "renders successfully for own person" do
+        get person_path(regular_user.person)
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    context "as an admin" do
+      before { sign_in admin }
+
+      it "renders successfully" do
+        get person_path(other_person)
+        expect(response).to have_http_status(:ok)
+      end
+    end
+  end
+end