ROCK-9044: Close connect-gate enforcement gaps and fix review findings

[stphnlee]

Summary

Follow-up to the ROCK-8640 Safety & Security connect gate (#8). Fixes all 13 findings from the code review of the v16 port. JIRA: ROCK-9044.

Enforcement (the reason this exists)

• Set up CI with Azure Pipelines #1 Bulk update was ungated. BulkUpdateRequests offered and wrote every state, including Connected, with no gate. New "Safety & Security Role" block setting; on confirm, if State = Connected, every request is checked with the shared gate against the target opportunity and status before anything is written. Any refusal hides the confirm modal and shows a Danger notice. Nothing persisted.
• Bug csf 12.6 getmatrixattributevalue #2 Cross-opportunity connect. Card-menu and modal Connect now require the request to be in the selected opportunity (IsRequestInSelectedOpportunity, fails closed). The edit check runs against the selected opportunity and the request id is client-supplied, so a forged connect|<id> could pass edit via one opportunity and the gate via another. Transfer already moves the board selection with the request, so transfer-then-connect still works.
• ROCK-8687 Fix Group Role Name not populating on next-gen check-in labels #3 Save gated against the wrong opportunity. Edit-modal save now evaluates the gate with the selected opportunity and selected status.

Crashes and feedback

• ROCK-8640 Hide ungated Connect action on Connection Request Board cards #6 Opportunity resolved and null-checked before the edit check that dereferenced it. No more yellow screen when the selected opportunity is deactivated mid-session.
• ROCK-8640 Hide ungated Connect action on Connection Request Board cards #7 Refused connects show "You are not authorized to connect this request." Board error box on the card path (same box the delete refusal uses), modal notice on the modal path.

Display

• SECC Connect Button Visibility Gate #4 Card-menu gate list is evaluated with no request in context via a new CanUserEditConnectionRequest( ConnectionRequest ) overload called with null. Previously one request's per-request Edit result was applied to every card, and the request-dependent list destabilised the client options hash (full re-render + flicker).
• Merge pull request #4 from secc/feature-gm-ROCK8640-ConnectGate-v3 #5 _connectionRequest cache cleared after transfer so the modal gates against the new opportunity.
• ROCK-8640 Forward-port S&S connect gate card fixes to v16 #8 RefreshRequestCard() after a confirmed drop so Connect reflects the new status.

Cleanup

• Add explicit Newtonsoft.Json reference to Rock.Blocks for VS 2026 builds #10 New SeccConnectGateHelper.GetConnectableStatusIds evaluates the status-independent half of the gate once per board render instead of once per status. LoadAttributes only when Attributes == null.
• ROCK-9079 Fix workflow conditional field visibility broken by RegistrationTemplateFormField Id collision #11 GetGateConnectionOpportunity uses the request's ConnectionOpportunity navigation property (same as ConnectionRequestDetail) instead of a hand-rolled query on an unowned RockContext.
• ROCK-9079 Fix workflow conditional field visibility broken by RegistrationTemplateFormField Id collision #12 Helper forwarding wrapper collapsed to one expression.
• Fix interaction retention never running on large channels #13 Fabricated ConnectionState.Active in the status loop is gone with the loop.
• Fix "That Wasn't Supposed to Happen" on grid Excel export (Group Attendance, Content Channel Item List) #9 JS fetchAndRefreshCard captures the gate list before its early return, matching initialize.

Cherry-pick

git apply --check of this diff against the ROCK-8640 squash commit (010d9f70a6) alone: applies cleanly. No context lines from ROCK-9046 or ROCK-9138. Pipeline order: secc/ROCK-8640-connect-gate, then this.

Config after deploy

Set Safety & Security Role on the Bulk Update Requests block instance. Until set, gated opportunities are Rock-Administrator-only in bulk (fails closed, same as the board and detail blocks).

Verification

• aspnet_compiler precompile of RockWeb (Plugins excluded): zero C# errors.
• Not exercised at runtime. Suggested manual pass:
  1. Non-S&S user, gated opp: bulk update → State = Connected → confirm. Expect Danger notice, nothing changed.
  2. Same user: card-menu Connect and modal Connect on a gated card. Expect "not authorized" message.
  3. S&S user: the three actions above succeed.
  4. Connection type with Request Security on: click a card you can't edit; Connect still shows on cards you can.
  5. Transfer a request into a gated opp, then into an ungated one; Connect button state correct immediately.
  6. Drag a card into and out of a connectable status; Connect item updates without reload.
  7. Deactivate the selected opp in another tab, post back; no yellow screen.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🔵 Needs a closer look

Authorization-sensitive changes span several mutation paths and have not been exercised at runtime.

Review effort: Balanced
Findings: None

What changed in this PR

Closes Safety & Security connect-gate gaps across board and bulk-update workflows.

Changes:

• Enforces authorization against target opportunities and statuses.
• Refreshes card gate state after transfers and drag/drop.
• Consolidates and optimizes shared gate evaluation.

File	Description
connectionRequestBoard.js	Captures updated gate data before early return.
ConnectionRequestBoard.ascx.cs	Strengthens connect enforcement and refresh behavior.
BulkUpdateRequests.ascx.cs	Adds bulk Connected-state authorization.
SeccConnectGateHelper.cs	Refactors and optimizes shared gate logic.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jwakefield-secc]

Review of PR 19 (head 1fe1ccb557), verified locally

I ran this PR's code on a local Rock 1.16 site against DEV data, as a non-S&S connector on a gated opportunity (via impersonation) and as an admin. Every DEV change the tests made was reverted. Seven issues are confirmed. The first six need a small change each; the seventh is a feedback gap.

1. Editing an already-Connected request is refused (ConnectionRequestBoard.ascx.cs, save handler ~L1577)
The edit modal only offers Connected, preselected, when the request is already Connected. The new save gate refuses any save with State = Connected, so a non-S&S editor can't save any change (comments, connector) to a connected request on a gated opportunity. They get "You are not authorized to connect this request." DEV has about 31,000 Connected requests on gated opportunities.
Fix: only gate when oldConnectionState != ConnectionState.Connected.

2. A forged request ID can still be acted on in another opportunity (CanUserEditConnectionRequest(ConnectionRequest) ~L2939)
A view|<id> command for a request in another opportunity opens its modal, and Edit is evaluated against the selected opportunity. So Transfer, Add Activity and connector reassignment run against a request in an opportunity the user has no rights on. The new IsRequestInSelectedOpportunity check only covers the two connect paths and the save.
Fix: do the opportunity-match check inside CanUserEditConnectionRequest(ConnectionRequest) for persisted requests, so every request-scoped action gets it. The view path probably needs the same check.

3. A forged card drop changes any request's status (card-drop-confirmed branch ~L885)
ProcessConfirmedDragEvent has no edit check and no opportunity check. A card-drop-confirmed|<id>|<statusId>|0 command changed the status of a request in an opportunity the user has no rights on, and renumbered both columns. This is stock behavior, but status now feeds the connect gate, and this PR already edits the branch.
Fix: if ( !IsRequestInSelectedOpportunity() || !CanUserEditConnectionRequest() ) return; before ProcessConfirmedDragEvent.

4. A modal refusal message carries over to the next request (btnRequestModalViewModeConnect_Click, save refusals)
The new ShowRequestModalNotification messages are never cleared when another request is opened, so the "not authorized" banner appears on an unrelated request. The PR resets nbNotificationBox per postback, but not the modal's box.
Fix: call HideRequestModalNotification() when a request modal is opened (ShowRequestModal / the view path).

5. Bulk moves of Connected requests skip the target's ConnectableStatuses (BulkUpdateRequests.ascx.cs ~L438)
The bulk gate passes each request's current state, so the helper's || connectionState == Connected exemption lets already-Connected requests into a gated opportunity at a status it doesn't allow connecting at. Confirmed by calling the helper directly, for an S&S member and a non-connectable status. 79 gated opportunities use ConnectableStatuses.
Fix: when the opportunity changes, evaluate the target status as a new connect, without the Connected exemption.

6. Bulk updates are refused for requests already Connected (BulkUpdateRequests.ascx.cs ~L434)
With State = Connected selected, requests that are already Connected on the same opportunity are gated too. A non-S&S user's whole batch is refused ("not authorized to connect N … No requests were updated") even though no request changes state.
Fix: skip requests where cr.ConnectionState == Connected && cr.ConnectionOpportunityId == targetOpportunityId.

7. A refused connect from the card menu is silent (card-menu connect ~L870, MarkRequestConnected)
When MarkRequestConnected itself refuses (the archived placement-group check), the message goes to the hidden modal's box. The card shows nothing, and the message then surfaces in the next modal opened.
Fix: on the card path, show that refusal with ShowError (the board box), or have MarkRequestConnected return a result that each caller can show in the right place.

Checked, no change needed here

• No practical workflow route around the gate: none of the workflow types that set a request to Connected is attached to a gated opportunity.
• The card-menu gate's opportunity-level Edit evaluation doesn't matter today: the only connection type with Request Security has no gated opportunities.
• The bulk null-opportunity crash is effectively unreachable: the opportunity list is always bound from active opportunities.

🤖 Generated with Claude Code

[jwakefield-secc]

Left some thoughts below on what would be helpful to fix post a local run of this branch.

[stphnlee]

Agreed on #1, #4, #5, #6, fixing in this PR. #1 was a regression I introduced. For #4 I'm hiding the box at the request-open entry points, not inside ShowRequestModal, because the modal connect path calls that right after MarkRequestConnected. #2 and #3 are stock IDORs and don't get around the gate, so they're going in a separate hardening ticket. #7 is ROCK-9138 code and goes in a follow-up there