Scope organization, partner user, and attachment lookups to the current organization

app/controllers/application_controller.rb

@@ -29,7 +29,9 @@ def current_organization
       nil
     elsif current_role.resource.is_a?(Organization)
       current_role.resource
-    else
+    elsif current_role.name.to_sym == Role::SUPER_ADMIN
+      # Super admins have no organization on their role, so they may choose
+      # which organization to act on. No other role may do this.
       Organization.find_by(id: params[:organization_id])
     end
   end

app/controllers/attachments_controller.rb

@@ -1,6 +1,13 @@
 class AttachmentsController < ApplicationController
+  # Only partner documents are deleted through this controller, and only for
+  # partners belonging to the current organization.
   def destroy
-    ActiveStorage::Attachment.find(params[:id])&.purge
+    ActiveStorage::Attachment.find_by!(
+      id: params[:id],
+      name: "documents",
+      record_type: "Partner",
+      record_id: current_organization.partners.select(:id)
+    ).purge
 
     redirect_back_or_to(partners_path)
   end

app/controllers/organizations_controller.rb

@@ -30,14 +30,14 @@ def invite_user
     UserInviteService.invite(email: params[:email],
       name: params[:name],
       roles: [Role::ORG_USER],
-      resource: Organization.find(params[:org]))
+      resource: current_organization)
     redirect_to organization_path, notice: "User invited to organization!"
   rescue => e
     redirect_to organization_path, alert: e.message
   end
 
   def resend_user_invitation
-    user = User.find(params[:user_id])
+    user = current_organization.users.find(params[:user_id])
     user.invite!
     redirect_to organization_path, notice: "User re-invited to organization!"
   end

app/controllers/partner_users_controller.rb

@@ -2,7 +2,7 @@
 
 class PartnerUsersController < ApplicationController
   before_action :authorize_admin
-  before_action :set_partner, only: %i[index create destroy resend_invitation]
+  before_action :set_partner
 
   def index
     @users = @partner.users
@@ -29,7 +29,7 @@ def create
   end
 
   def destroy
-    user = User.find(params[:id])
+    user = @partner.users.find(params[:id])
 
     if user.remove_role(Role::PARTNER, @partner)
       redirect_back_or_to("/", notice: "Access to #{@partner.name} has been revoked for #{user.display_name}.")
@@ -39,7 +39,7 @@ def destroy
   end
 
   def resend_invitation
-    user = User.find(params[:id])
+    user = @partner.users.find(params[:id])
 
     if user.invitation_accepted_at.nil?
       user.invite!
@@ -55,7 +55,7 @@ def resend_invitation
   end
 
   def reset_password
-    user = User.find(params[:id])
+    user = @partner.users.find(params[:id])
 
     user.send_reset_password_instructions
     redirect_back_or_to(root_path, notice: "Password e-mail sent!")
@@ -64,7 +64,7 @@ def reset_password
   private
 
   def set_partner
-    @partner = Partner.find(params[:partner_id])
+    @partner = current_organization.partners.find(params[:partner_id])
   end
 
   def user_params

app/views/users/_add_user_modal.erb

@@ -17,7 +17,7 @@
               <span class="input-group-text" id="spn_env_fa_icon"><%= fa_icon "envelope" %></span>
               <input type="email" name="email" class="form-control" placeholder="Email" aria-describedby="spn_env_fa_icon">
             </div>
-            <%= hidden_field_tag :org , current_organization.id %><br>
+            <%= hidden_field_tag :organization_id, current_organization.id %><br>
             <%= submit_button({ text: "Invite User", icon: "envelope" }) %>
           <% end # form %>
         </div><!-- box-body -->

spec/controllers/application_controller_spec.rb

@@ -12,6 +12,20 @@
       it "should return nil" do
         expect(controller.current_organization).to eq(nil)
       end
+
+      it "should return nil even when an organization_id param is given" do
+        controller.params[:organization_id] = organization.id
+        expect(controller.current_organization).to eq(nil)
+      end
+    end
+
+    context "As a super admin" do
+      let(:user) { create(:super_admin, organization: nil) }
+
+      it "should return the organization from the organization_id param" do
+        controller.params[:organization_id] = organization.id
+        expect(controller.current_organization).to eq(organization)
+      end
     end
 
     context "As an org user" do

spec/requests/attachments_requests_spec.rb

@@ -1,14 +1,32 @@
 RSpec.describe "Attachments", type: :request do
+  let(:organization) { create(:organization) }
+
   before do
-    sign_in(create(:user))
+    sign_in(create(:user, organization: organization))
   end
 
   describe "DELETE #destroy" do
-    let(:partner) { create(:partner, documents: [Rack::Test::UploadedFile.new(Rails.root.join("spec/fixtures/files/dbase.pdf"), "application/pdf")]) }
+    let(:partner) { create(:partner, organization: organization, documents: [Rack::Test::UploadedFile.new(Rails.root.join("spec/fixtures/files/dbase.pdf"), "application/pdf")]) }
 
     it "redirects to referrer" do
       delete attachment_path(partner.documents.first)
       expect(response).to redirect_to(partners_path)
     end
+
+    it "does not delete another organization's partner documents" do
+      other_partner = create(:partner, organization: create(:organization), documents: [Rack::Test::UploadedFile.new(Rails.root.join("spec/fixtures/files/dbase.pdf"), "application/pdf")])
+      attachment = other_partner.documents.first
+      delete attachment_path(attachment)
+      expect(response).to have_http_status(:not_found)
+      expect(ActiveStorage::Attachment.exists?(attachment.id)).to be(true)
+    end
+
+    it "does not delete attachments that are not partner documents" do
+      organization.logo.attach(io: Rails.root.join("spec/fixtures/files/logo.jpg").open, filename: "logo.jpg")
+      attachment = organization.logo.attachment
+      delete attachment_path(attachment)
+      expect(response).to have_http_status(:not_found)
+      expect(ActiveStorage::Attachment.exists?(attachment.id)).to be(true)
+    end
   end
 end

spec/requests/dashboard_requests_spec.rb

@@ -59,6 +59,17 @@
     end
   end
 
+  context "While signed in as a partner user" do
+    before do
+      sign_in(create(:partner_user, partner: create(:partner, organization: organization)))
+    end
+
+    it "does not allow choosing an organization via organization_id" do
+      get dashboard_path(organization_id: create(:organization).id)
+      expect(response).to redirect_to(partners_dashboard_path)
+    end
+  end
+
   context "While not signed in" do
     it "redirects for authentication" do
       get dashboard_path

spec/requests/organization_requests_spec.rb

@@ -490,12 +490,34 @@
       end
     end
 
+    describe "POST #invite_user" do
+      it "invites the user to the admin's organization" do
+        post invite_user_organization_path(email: "new_user@example.com", name: "New User")
+        expect(User.find_by(email: "new_user@example.com").has_role?(Role::ORG_USER, organization)).to be(true)
+      end
+    end
+
     context "when attempting to access a different organization" do
       let(:other_organization) { create(:organization) }
       let(:other_organization_params) do
         { organization_id: other_organization.id }
       end
 
+      describe "POST #invite_user" do
+        it "does not invite to the other organization" do
+          post invite_user_organization_path(org: other_organization.id, email: organization_admin.email)
+          expect(organization_admin.reload.has_role?(Role::ORG_USER, other_organization)).to be(false)
+        end
+      end
+
+      describe "POST #resend_user_invitation" do
+        it "does not re-invite the other organization's user" do
+          other_user = create(:user, organization: other_organization)
+          post resend_user_invitation_organization_path(user_id: other_user.id)
+          expect(response).to have_http_status(:not_found)
+        end
+      end
+
       describe "GET #show" do
         before { get organization_path(other_organization_params) }
 
@@ -532,6 +554,14 @@
       sign_in(super_admin_org_admin)
     end
 
+    describe "POST #invite_user" do
+      it "invites the user to the organization given by organization_id" do
+        other_organization = create(:organization)
+        post invite_user_organization_path(organization_id: other_organization.id, email: "new_user@example.com")
+        expect(User.find_by(email: "new_user@example.com").has_role?(Role::ORG_USER, other_organization)).to be(true)
+      end
+    end
+
     describe "POST #promote_to_org_admin" do
       context "promoting a user" do
         include_examples "promote to admin check", :user, :super_admin

spec/requests/partner_users_requests_spec.rb

@@ -1,8 +1,8 @@
 # spec/requests/partner_users_controller_spec.rb
 
 RSpec.describe PartnerUsersController, type: :request do
-  let!(:partner) { create(:partner) } # Assuming you have a factory for creating partners
   let(:organization) { create(:organization) }
+  let!(:partner) { create(:partner, organization: organization) }
   let(:user) { create(:user, organization: organization) }
   let(:org_admin) { create(:organization_admin, organization: organization) }
   let(:default_params) do
@@ -236,4 +236,42 @@
       end
     end
   end
+
+  context "with a partner from another organization" do
+    let(:other_partner) { create(:partner, organization: create(:organization)) }
+    let!(:other_partner_user) { create(:partner_user, partner: other_partner) }
+
+    before do
+      sign_in(org_admin)
+    end
+
+    it "does not list its users" do
+      get partner_users_path(partner_id: other_partner)
+      expect(response).to have_http_status(:not_found)
+    end
+
+    it "does not invite users to it" do
+      post partner_users_path(partner_id: other_partner), params: {user: {email: org_admin.email, name: "Admin"}}
+      expect(response).to have_http_status(:not_found)
+      expect(org_admin.reload.has_role?(Role::PARTNER, other_partner)).to be(false)
+    end
+
+    it "does not remove its users" do
+      delete partner_user_path(partner_id: other_partner, id: other_partner_user)
+      expect(response).to have_http_status(:not_found)
+      expect(other_partner_user.reload.has_role?(Role::PARTNER, other_partner)).to be(true)
+    end
+
+    it "does not resend invitations to its users" do
+      post resend_invitation_partner_user_path(partner_id: other_partner, id: other_partner_user)
+      expect(response).to have_http_status(:not_found)
+    end
+
+    it "does not reset its users' passwords via one of your own partners" do
+      expect {
+        post reset_password_partner_user_path(partner_id: partner, id: other_partner_user)
+      }.not_to change { ActionMailer::Base.deliveries.count }
+      expect(response).to have_http_status(:not_found)
+    end
+  end
 end

spec/requests/users_requests_spec.rb

@@ -29,7 +29,7 @@
 
   describe "POST #partner_user_reset_password" do
     let(:partner) { create(:partner, organization: organization) }
-    let!(:user) { create(:partner_user, partner: partner, email: "me@partner.com") }
+    let!(:partner_user) { create(:partner_user, partner: partner, email: "me@partner.com") }
     let(:params) { { organization_id: organization.id, partner_id: partner.id, email: "me@partner.com" } }
 
     it "should send a password" do
@@ -46,13 +46,13 @@
     end
 
     it "should return an error if it cannot find the user" do
-      user.update!(email: "some-other-email@mail.com")
+      partner_user.update!(email: "some-other-email@mail.com")
       post partner_user_reset_password_users_path(params)
       expect(ActionMailer::Base.deliveries.size).to eq(0)
     end
 
     it "should return send a password even if case-insensitive spelling of email" do
-      user.update!(email: "Me@partner.com")
+      partner_user.update!(email: "Me@partner.com")
       post partner_user_reset_password_users_path(params)
       expect(ActionMailer::Base.deliveries.size).to eq(1)
     end