fix(authz): block non-admin members from changing duration on published events

[JpMaxMan]

Summary

Move the published-event duration guard into SummitEvent::setDuration so the invariant is enforced at the entity boundary, not just in the UI or a single controller.

Why

For a published event, SummitEvent::setDuration → TimeDurationRestrictedEvent::_setDuration silently shifts end_date = start_date + duration whenever start_date is set. The track-chair edit path (PUT /api/v1/summits/{id}/events/{event_id} → OAuth2SummitEventsApiController@updateEvent → AbstractPublishService::updateDuration) calls setDuration(..., $current_member) without checking publication state, which means a chair token + curl can move a live schedule slot with no audit and no notification.

Putting the check in the controller would close the chair path but leave the invariant scattered: any future service or controller that calls setDuration would need to remember to repeat the check. The setter is the single place every caller must pass through, so the entity should own the rule.

What changed

• app/Models/Foundation/Summit/Events/SummitEvent.php — setDuration now throws ValidationException when the event is published and the supplied $member is non-admin for the summit.
• tests/SummitEventModelTest.php — adds two cases against an already-published presentation fixture: a mocked non-admin member is rejected, a mocked admin member succeeds and the end_date shifts as before.

Compatibility notes

• The check intentionally skips when $member is null. Trusted internal callers like SummitService::advanceSummit (app/Services/Model/Imp/SummitService.php:2731) call setDuration with no member during bulk schedule shifts and must continue to work. That mirrors the convention used elsewhere where authorization checks are gated on a user being present.
• The existing testChangeEventDuration continues to pass because it also calls setDuration without a member.
• This is the API-side complement to the UI gate landed in fntechgit/track-chairs#67. They are independent — either alone closes the visible workflow, but the entity guard is what protects against direct API calls.

Test plan

• phpunit tests/SummitEventModelTest.php — new tests pass; existing tests still pass.
• Manual: as a track chair, PUT /api/v1/summits/{id}/events/{published_event_id} with {"duration": 1800} returns a 412 ValidationException response (was: 200 with the slot's end_date silently moved).
• Manual: as a summit admin, the same call still succeeds.
• Manual: scheduling/advance-summit bulk operations continue to update durations on published events.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

• Enhanced authorization controls for published summit event duration modifications. The system now validates member permissions before allowing duration changes on published events. Non-authorized members cannot modify published event durations and will receive validation errors on attempt. This prevents unauthorized schedule changes while allowing authorized members to adjust event times as needed.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds authorization validation to SummitEvent::setDuration(). When a member attempts to change duration on a published event, the method now checks if that member is allowed for the summit. Non-allowed members receive a ValidationException; allowed members proceed normally. Test coverage validates both authorization paths.

Changes

Published Event Duration Authorization

Layer / File(s)	Summary
Core Authorization Logic app/Models/Foundation/Summit/Events/SummitEvent.php	SummitEvent::setDuration() adds a guard that throws ValidationException if a published event is modified by a member not allowed for the summit.
Test Coverage & Imports tests/SummitEventModelTest.php	Test file imports ValidationException and adds two test cases: one verifying non-allowed member raises exception, another verifying allowed member updates duration and advances end date.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hops through authorization's gate,
Checking members swift, not letting them wait—
Published events now guarded with care,
Only summit-allowed may dare
To change what duration must bear! 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(authz): block non-admin members from changing duration on published events' directly and accurately reflects the main change: adding authorization checks to prevent non-admin members from modifying published event durations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/track-chair-cannot-modify-published-duration

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

📘 OpenAPI / Swagger preview

➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-538/

This page is automatically updated on each push to this PR.

[coderabbitai]

🧹 Nitpick comments (1)

tests/SummitEventModelTest.php (1)

83-105: ⚡ Quick win

Add a regression test for the null-member compatibility path.

Given the documented behavior, add a test that setDuration(..., null) still succeeds on published events for trusted internal flows; this protects against future hardening regressions.

Proposed test addition

+    public function testNullMemberCanChangeDurationOnPublishedEvent(){
+        $presentation = self::$presentations[0];
+        $this->assertTrue($presentation->isPublished());
+
+        $old_end_date = $presentation->getEndDate();
+        $presentation->setDuration(864000, false, null);
+        $new_end_date = $presentation->getEndDate();
+
+        $this->assertTrue($old_end_date < $new_end_date);
+    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/SummitEventModelTest.php` around lines 83 - 105, Add a new regression
test method in SummitEventModelTest that mirrors
testAdminMemberCanChangeDurationOnPublishedEvent but passes null for the $member
argument to Presentation::setDuration to exercise the null-member compatibility
path: grab a published presentation (self::$presentations[0]), assert
isPublished(), record old end date, call $presentation->setDuration(864000,
false, null) and assert no exception and that getEndDate() increased (old <
new). Ensure the test name clearly indicates null-member behavior (e.g.,
testNullMemberCanChangeDurationOnPublishedEvent) and follows the same
mocking-free flow as the admin test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/SummitEventModelTest.php`:
- Around line 83-105: Add a new regression test method in SummitEventModelTest
that mirrors testAdminMemberCanChangeDurationOnPublishedEvent but passes null
for the $member argument to Presentation::setDuration to exercise the
null-member compatibility path: grab a published presentation
(self::$presentations[0]), assert isPublished(), record old end date, call
$presentation->setDuration(864000, false, null) and assert no exception and that
getEndDate() increased (old < new). Ensure the test name clearly indicates
null-member behavior (e.g., testNullMemberCanChangeDurationOnPublishedEvent) and
follows the same mocking-free flow as the admin test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d22467f-83d1-41a2-962f-a71cc3209245

📥 Commits

Reviewing files that changed from the base of the PR and between 34e47f9 and 0fd1601.

📒 Files selected for processing (2)

• app/Models/Foundation/Summit/Events/SummitEvent.php
• tests/SummitEventModelTest.php

[smarcet]

LGTM

[romanetar]

LGTM