From 05d10fe11991dfadf2f11ee998799bc19d3fb3c8 Mon Sep 17 00:00:00 2001 From: maebeale Date: Thu, 19 Feb 2026 10:28:30 -0500 Subject: [PATCH 1/3] Restrict people and organization access to admins only Temporarily lock down index and show for Person and Organization policies to admin-only while these features are prepared for separate launch. Co-Authored-By: Claude Opus 4.6 --- app/policies/organization_policy.rb | 4 ++-- app/policies/person_policy.rb | 4 ++-- spec/policies/person_policy_spec.rb | 12 +++--------- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/app/policies/organization_policy.rb b/app/policies/organization_policy.rb index 4dd0dd841..d8c67b6cf 100644 --- a/app/policies/organization_policy.rb +++ b/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? diff --git a/app/policies/person_policy.rb b/app/policies/person_policy.rb index a01414d30..f3befe0a2 100644 --- a/app/policies/person_policy.rb +++ b/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? end def edit? diff --git a/spec/policies/person_policy_spec.rb b/spec/policies/person_policy_spec.rb index c958e4c9e..937a54ef2 100644 --- a/spec/policies/person_policy_spec.rb +++ b/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 @@ -43,18 +43,12 @@ def policy_for(record: nil, user:) context "with owner" do subject { policy_for(record: owned_person, user: owner_user) } - it { is_expected.to be_allowed_to(:show?) } + it { is_expected.not_to be_allowed_to(:show?) } end - context "with regular user and searchable person" do + context "with regular user" do subject { policy_for(record: searchable_person, user: regular_user) } - it { is_expected.to be_allowed_to(:show?) } - end - - context "with regular user and non-searchable person" do - subject { policy_for(record: non_searchable_person, user: regular_user) } - it { is_expected.not_to be_allowed_to(:show?) } end From 75bd7649ed83bfc93b44de3113c626da110428dd Mon Sep 17 00:00:00 2001 From: maebeale Date: Thu, 19 Feb 2026 10:31:26 -0500 Subject: [PATCH 2/3] Keep both searchable/non-searchable test cases for person show policy Co-Authored-By: Claude Opus 4.6 --- spec/policies/person_policy_spec.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/policies/person_policy_spec.rb b/spec/policies/person_policy_spec.rb index 937a54ef2..9a2120dc4 100644 --- a/spec/policies/person_policy_spec.rb +++ b/spec/policies/person_policy_spec.rb @@ -46,12 +46,18 @@ def policy_for(record: nil, user:) it { is_expected.not_to be_allowed_to(:show?) } end - context "with regular user" do + context "with regular user and searchable person" do subject { policy_for(record: searchable_person, user: regular_user) } it { is_expected.not_to be_allowed_to(:show?) } end + context "with regular user and non-searchable person" do + subject { policy_for(record: non_searchable_person, user: regular_user) } + + it { is_expected.not_to be_allowed_to(:show?) } + end + context "with no user" do subject { policy_for(record: searchable_person, user: nil) } From 518d1b41eec8dce2a441d237a797e61c9bfa154b Mon Sep 17 00:00:00 2001 From: maebeale Date: Thu, 19 Feb 2026 11:40:37 -0500 Subject: [PATCH 3/3] Add authorization specs and fix person show for owners - Allow owners to view their own person show page (PersonPolicy#show?) - Fix show action to authorize before decorating (Draper was masking owner? check) - Add organization_policy_spec.rb - Add people and organizations authorization request specs - Cover visitor, regular user, owner, and admin access patterns Co-Authored-By: Claude Opus 4.6 --- app/controllers/people_controller.rb | 2 +- app/policies/person_policy.rb | 2 +- spec/policies/organization_policy_spec.rb | 86 +++++++++++++++++++ spec/policies/person_policy_spec.rb | 2 +- .../organizations_authorization_spec.rb | 63 ++++++++++++++ spec/requests/people_authorization_spec.rb | 66 ++++++++++++++ 6 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 spec/policies/organization_policy_spec.rb create mode 100644 spec/requests/organizations_authorization_spec.rb create mode 100644 spec/requests/people_authorization_spec.rb diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 073ea48a7..9f31ce0b4 100644 --- a/app/controllers/people_controller.rb +++ b/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 diff --git a/app/policies/person_policy.rb b/app/policies/person_policy.rb index f3befe0a2..0c96d358c 100644 --- a/app/policies/person_policy.rb +++ b/app/policies/person_policy.rb @@ -6,7 +6,7 @@ def index? end def show? - admin? + admin? || owner? end def edit? diff --git a/spec/policies/organization_policy_spec.rb b/spec/policies/organization_policy_spec.rb new file mode 100644 index 000000000..a66e8d94a --- /dev/null +++ b/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 diff --git a/spec/policies/person_policy_spec.rb b/spec/policies/person_policy_spec.rb index 9a2120dc4..41a49d194 100644 --- a/spec/policies/person_policy_spec.rb +++ b/spec/policies/person_policy_spec.rb @@ -43,7 +43,7 @@ def policy_for(record: nil, user:) context "with owner" do subject { policy_for(record: owned_person, user: owner_user) } - it { is_expected.not_to be_allowed_to(:show?) } + it { is_expected.to be_allowed_to(:show?) } end context "with regular user and searchable person" do diff --git a/spec/requests/organizations_authorization_spec.rb b/spec/requests/organizations_authorization_spec.rb new file mode 100644 index 000000000..02072f75d --- /dev/null +++ b/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 diff --git a/spec/requests/people_authorization_spec.rb b/spec/requests/people_authorization_spec.rb new file mode 100644 index 000000000..ebef4a307 --- /dev/null +++ b/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