Skip to content

Conversation

@manuthecoder
Copy link
Contributor

@manuthecoder manuthecoder commented Jun 5, 2025

This pull request introduces changes to enhance authorization checks and ensure proper role-based access control across policies and views. The updates primarily focus on restricting actions to users with the appropriate roles, such as organizers with at least the :member role, and improving the handling of deletions.

Authorization Enhancements:

  • Updated OrganizerPositionPolicy to allow record owners to destroy positions in addition to admins or contract signees. (app/policies/organizer_position_policy.rb, app/policies/organizer_position_policy.rbL5-R5)
  • Modified ReceiptPolicy to use role-based checks (OrganizerPosition.role_at_least?) for destruction permissions instead of relying on event user inclusion. (app/policies/receipt_policy.rb, app/policies/receipt_policy.rbL6-R6)

Role-Based View Access:

  • Added role-specific checks (organizer_signed_in?(as: :member)) for rendering "Add tag" buttons, links, and modals across multiple views, including canonical_pending_transactions, canonical_transactions, employees, and events views. [1] [2] [3] [4] [5] [6] [7]
  • Ensured that voiding or archiving invoices in hcb_codes/_invoice.html.erb is restricted to organizers with at least the :member role.

Improved Deletion Handling:

  • Updated deletion logic in receipts/_receipt.html.erb to check policy(receipt).destroy? before rendering delete buttons, ensuring only authorized users can delete receipts. [1] [2]

@manuthecoder manuthecoder marked this pull request as ready for review June 12, 2025 20:55
@manuthecoder manuthecoder requested review from a team as code owners June 12, 2025 20:55
@manuthecoder manuthecoder force-pushed the improve-reader-ui-part-2 branch from df09aaa to c711c96 Compare July 3, 2025 00:19
@manuthecoder manuthecoder requested a review from sampoder July 3, 2025 00:25
Copy link
Contributor

@Luke-Oldenburg Luke-Oldenburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at #10776 for some similar / duplicate fixes that use more policy instead.

@garyhtou garyhtou linked an issue Jul 15, 2025 that may be closed by this pull request
Copy link
Contributor

@Luke-Oldenburg Luke-Oldenburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there just a few small changes. I reckon the PR title should be updated to be more general too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should freeze cards instead of cancel and be in a separate PR please.

Comment on lines +198 to +203
class: "btn bg-error mr1 right" if @invoice.open_v2? && organizer_signed_in?(as: :member) %>
<%= link_to (@invoice.archived? ? "Un-archive" : "Archive"),
(@invoice.archived? ? invoice_unarchive_path(@invoice) : invoice_archive_path(@invoice)),
disabled: !organizer_signed_in?,
method: :post,
class: "btn bg-muted" unless @invoice.void_v2? %>
class: "btn bg-muted" unless @invoice.void_v2? || !organizer_signed_in?(as: :member) %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make policies for these please?

class OrganizerPositionPolicy < ApplicationPolicy
def destroy?
admin_or_contract_signee?
admin_or_contract_signee? || record.user == user
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be in a separate PR since it's not strictly UI related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Roles] Freeze cards when demoted to reader

3 participants