Skip to content

Cfodev 1669 review archival rules who and when can archive#1148

Merged
Carl Sixsmith (carlsixsmith-moj) merged 3 commits into
developfrom
CFODEV-1669-review-archival-rules-who-and-when-can-archive
Jun 21, 2026
Merged

Cfodev 1669 review archival rules who and when can archive#1148
Carl Sixsmith (carlsixsmith-moj) merged 3 commits into
developfrom
CFODEV-1669-review-archival-rules-who-and-when-can-archive

Conversation

@vks333

@vks333 Vinay Saini (vks333) commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

This pull request introduces a new business rule to control who is authorized to archive a participant, ensuring that only the participant's owner or users from the same or higher-level tenant can perform the archive action. It updates both backend and frontend logic to enforce this rule, adds relevant data to DTOs, and includes comprehensive unit tests for the new rule and its effects.

Authorization and Business Rule Enforcement

  • Added a new business rule class ParticipantMustBeArchivableByUser to enforce that only the participant's owner or users from the same or higher-level tenant can archive a participant.
  • Refactored the ArchiveCase command handler to use the new Archive method, which applies the business rule, and now requires ICurrentUserService for user context. [1] [2] [3]
  • Implemented the Archive method in the Participant entity, which checks the business rule before allowing the status transition.

Frontend and DTO Updates

  • Updated ParticipantSummaryDto and related mapping to include OwnerId and OwnerTenantId, enabling the frontend to determine archive permissions. [1] [2] [3]
  • Modified the participant action menu UI to only show the "Archive" option if the current user is authorized according to the new rule.

Testing

  • Added unit tests for ParticipantMustBeArchivableByUser covering various scenarios (owner, higher/same/lower tenant, different branch).
  • Added tests to verify that unauthorized users cannot archive a participant and that authorized users can. [1] [2]

🔗 Related Work

  • Closes: #
  • Related: #

📌 Summary

What does this PR do? Keep it short but clear.

---Unauthorised users should not be able to archive i.e. if they are not case owner or are not at same level as owner's tenant or higher.

🎯 Purpose / Motivation

Why does this change exist? What problem are we solving?


🧠 Approach

Key implementation details, trade-offs, or design decisions.
Mention anything reviewers should pay attention to.

---As pipeline runs validation before authorization, so chose to move access checks in validators to handler because they are better enforced in the authorization path

🔄 Changes

  • Added:
  • Updated:
  • Removed:

🧪 How to Test

Step-by-step instructions to verify this works.

  1. As case owner or at the same tenant level as owner or above I should be able to archive a case
  2. If logged in user is in different tenant than the case owner or at lower level tenant than the case owner they should be able to archive:
    a. Archive should be hidden
    b. (IT Only)To test if server side validations works, make Archive button visible by removing ParticipantArchiveAccess.CanArchive from ParticipantActionMenu.razor page and try to archive, it should show error message that "You are not authorized to archive this participant".

Expected result:

What should happen if everything is correct?


📸 Screenshots / Output (if applicable)

UI changes, logs, API responses, etc.


⚠️ Risks & Impact

  • Breaking change
  • Database change
  • Performance impact
  • Security impact

Details:

Anything reviewers should be cautious about.


🙋 Notes for Reviewers

Anything specific you want feedback on.
e.g. "Unsure about approach in X", "Focus on Y logic"

@vks333 Vinay Saini (vks333) requested review from a team as code owners June 18, 2026 12:15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a fan of random static classes enforcing business rule.

We have IBusinessRule that can implement the guard and offer the lowest level of protection at the aggregate level.

The UI button logic can be managed through a simple check of the users tenantid and the tenantid of the owner of the case.

Comment thread src/Application/Features/Participants/ParticipantArchiveAccess.cs Outdated
@carlsixsmith-moj Carl Sixsmith (carlsixsmith-moj) added the Changes Requested Changes have been requested for this pull request label Jun 18, 2026
@vks333 Vinay Saini (vks333) added Needs Review This pull request is awaiting review Feature This is a new feature (or extending an existing one) and removed Changes Requested Changes have been requested for this pull request labels Jun 18, 2026
…ints are consistently protected

-Removed validator and enforced in the authorisation path because pipeline runs validation before authorisation, so putting access checks in validators can return the wrong failure type and may leak participant-existence details
-Renamed ParticipantArchiveAccess.CanAccess to ParticipantArchiveAccess.CanArchive, was a bit repetitive
-updated tests accordingly
-Implemented this as a domain guard via IBusinessRule, authorisation now sits at aggregate level.
-Removed ParticipantArchiveAccess helper and a simple owner/tenant visibility check on UI
@carlsixsmith-moj Carl Sixsmith (carlsixsmith-moj) force-pushed the CFODEV-1669-review-archival-rules-who-and-when-can-archive branch from deb5494 to d5024f2 Compare June 21, 2026 07:35
@carlsixsmith-moj Carl Sixsmith (carlsixsmith-moj) merged commit ebe8d42 into develop Jun 21, 2026
1 check passed
@carlsixsmith-moj Carl Sixsmith (carlsixsmith-moj) deleted the CFODEV-1669-review-archival-rules-who-and-when-can-archive branch June 21, 2026 07:37
@carlsixsmith-moj Carl Sixsmith (carlsixsmith-moj) added Approved Changes have been approved and removed Needs Review This pull request is awaiting review labels Jun 21, 2026
Carl Sixsmith (carlsixsmith-moj) pushed a commit that referenced this pull request Jun 25, 2026
* Enforce archive authorization by owner or tenant hierarchy

* Enforced in the authorization path (or handler guard) so all entry points are consistently protected

-Removed validator and enforced in the authorisation path because pipeline runs validation before authorisation, so putting access checks in validators can return the wrong failure type and may leak participant-existence details
-Renamed ParticipantArchiveAccess.CanAccess to ParticipantArchiveAccess.CanArchive, was a bit repetitive
-updated tests accordingly

* Domain guard via IBusinessRule,

-Implemented this as a domain guard via IBusinessRule, authorisation now sits at aggregate level.
-Removed ParticipantArchiveAccess helper and a simple owner/tenant visibility check on UI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Changes have been approved Feature This is a new feature (or extending an existing one)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants