Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: sanitize attendee status on create and specific changes #47308

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

fix: sanitize attendee status on create and specific changes #47308

wants to merge 2 commits into from
+168 −0

Conversation

SebastianKrupinski
Copy link
Contributor

Summary

TODO

  • Tests

Checklist

github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 17, 2024
View reviewed changes
apps/dav/lib/CalDAV/Plugin.php
* @param mixed $modified
* @param mixed $isNew
*/
public function calendarObjectChange(RequestInterface $request, ResponseInterface $response, VCalendar $alteredObject, $calendarPath, &$modified, $isNew) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\DAV\CalDAV\Plugin::calendarObjectChange does not have a return type, expecting void
apps/dav/lib/CalDAV/Plugin.php
// determine if altered calendar event is a new
// if calendar event is new sanitize and exit
if ($isNew) {
$this->sanitizeCreatedInstance($alteredObject->VEVENT, $modified);

Check failure

Code scanning / Psalm

InvalidArgument Error

Argument 1 of OCA\DAV\CalDAV\Plugin::sanitizeCreatedInstance expects Sabre\VObject\Component\VEvent, but Sabre\VObject\Property provided
apps/dav/lib/CalDAV/Plugin.php
/** @var \Sabre\VObject\Component\VCalendar $currentObject */
$currentObject = Reader::read($currentNode->get());
// find what has changed (base, recurrence, both) between altered and current calendar event
$delta = $this->findEventInstanceDelta($alteredObject->VEVENT, $currentObject->VEVENT);

Check failure

Code scanning / Psalm

InvalidArgument Error

Argument 1 of OCA\DAV\CalDAV\Plugin::findEventInstanceDelta expects Sabre\VObject\Component\VEvent, but Sabre\VObject\Property provided
apps/dav/lib/CalDAV/Plugin.php
/** @var \Sabre\VObject\Component\VCalendar $currentObject */
$currentObject = Reader::read($currentNode->get());
// find what has changed (base, recurrence, both) between altered and current calendar event
$delta = $this->findEventInstanceDelta($alteredObject->VEVENT, $currentObject->VEVENT);

Check failure

Code scanning / Psalm

InvalidArgument Error

Argument 2 of OCA\DAV\CalDAV\Plugin::findEventInstanceDelta expects Sabre\VObject\Component\VEvent, but Sabre\VObject\Property|null provided
apps/dav/lib/CalDAV/Plugin.php

}

public function sanitizeCreatedInstance(VEvent $altered, $modified): void {

Check notice

Code scanning / Psalm

MissingParamType Note

Parameter $modified has no provided type
apps/dav/lib/CalDAV/Plugin.php
public function sanitizeEventAttendees(VEvent $event, $modified): void {

// iterate thought attendees
foreach ($event->ATTENDEE as $id => $entry) {

Check notice

Code scanning / Psalm

PossiblyNullIterator Note

Cannot iterate over nullable var Sabre\VObject\Property|null
apps/dav/lib/CalDAV/Plugin.php
// determine attendee participation status
// if status is missing or NOT set correctly change the status
if (!isset($entry['PARTSTAT']) || $entry['PARTSTAT']->getValue() !== 'NEEDS-ACTION') {
$event->ATTENDEE[$id]['PARTSTAT']->setValue('NEEDS-ACTION');

Check failure

Code scanning / Psalm

InvalidArgument Error

Argument 1 of Sabre\VObject\Node::offsetGet expects int, but 'PARTSTAT' provided
apps/dav/lib/CalDAV/Plugin.php
// determine if id exists in list
if (isset($list[$id])) {
// compare altered instance to current instance
if ($list[$id]['altered']->{'LAST-MODIFIED'}->getValue() == $event->{'LAST-MODIFIED'}->getValue() &&

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getValue on possibly null value
apps/dav/lib/CalDAV/Plugin.php
if (isset($list[$id])) {
// compare altered instance to current instance
if ($list[$id]['altered']->{'LAST-MODIFIED'}->getValue() == $event->{'LAST-MODIFIED'}->getValue() &&
$list[$id]['altered']->SEQUENCE->getValue() == $event->SEQUENCE->getValue()) {

Check notice

Code scanning / Psalm

PossiblyNullPropertyFetch Note

Cannot get property on possibly null variable $list[$id]['altered'] of type mixed|null
apps/dav/lib/CalDAV/Plugin.php
if (isset($list[$id])) {
// compare altered instance to current instance
if ($list[$id]['altered']->{'LAST-MODIFIED'}->getValue() == $event->{'LAST-MODIFIED'}->getValue() &&
$list[$id]['altered']->SEQUENCE->getValue() == $event->SEQUENCE->getValue()) {

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getValue on possibly null value
@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Aug 17, 2024 •
edited
Loading

Testing Steps

Create Test

  1. Using external tool PUT a new event with invalid attendee participant status e.g (Accepted)

Attendee participation status should be reset to "NEEDS-ACTION"

Update Test

  1. Create a singleton event with attendees.
  2. Accept event as attendee.
  3. Change event Start, End, Location or Recurrence.
  4. Repeat 1 to 3 with a recurring event

Attendee participation status should be reset to "NEEDS-ACTION"

@SebastianKrupinski SebastianKrupinski added the 2. developing Work in progress label Aug 17, 2024
@SebastianKrupinski SebastianKrupinski self-assigned this Aug 17, 2024
@tcitworld
Copy link
Member

For reference: #42347

@ChristophWurst ChristophWurst marked this pull request as draft August 19, 2024 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@miaulalala miaulalala Awaiting requested review from miaulalala

@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

@st3iny st3iny Awaiting requested review from st3iny st3iny will be requested when the pull request is marked ready for review st3iny is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@SebastianKrupinski SebastianKrupinski

Labels
2. developing Work in progress blocked
Projects
💌 📅 👥 Groupware team
Status: 🏗️ In progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

After organizer moves event, attendee status is still "accepted", should be "needs-action"
2 participants
@SebastianKrupinski @tcitworld