Skip to content

Conversation

@SpechtD
Copy link
Contributor

@SpechtD SpechtD commented Mar 16, 2022

closes #162

Copy link
Member

@sualko sualko left a comment

Choose a reason for hiding this comment

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

Looks good, but needs some changes until we can merge it.

bool $cleanLayout,
bool $joinMuted
bool $joinMuted,
string $logoutURL
Copy link
Member

Choose a reason for hiding this comment

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

We need some type of validation for the logoutURL. At least it has to start with http:// or https://.

Copy link
Member

Choose a reason for hiding this comment

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

This validation should also be done on the server side, otherwise an attacker could bypass the client logic.

return new DataResponse(['message' => 'Not allowed to enable recordings.'], Http::STATUS_BAD_REQUEST);
}

if (!$restriction->getallowLogoutURL() && $logoutURL !== $room->getlogoutURL()) {
Copy link
Member

Choose a reason for hiding this comment

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

Is the value of allowLogoutURL determined somewhere? You should probably look into the Permission class 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The value of allowLogoutURL is determined in the Restriction class, together with the other restrictions.

$restriction1->setMaxRooms(10);
$restriction1->setMaxParticipants(100);
$restriction1->setAllowRecording(true);
$restriction1->setAllowRecording(true);
Copy link
Member

Choose a reason for hiding this comment

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

Should probably setAllowLogoutURL.

bool $cleanLayout,
bool $joinMuted
bool $joinMuted,
string $logoutURL
Copy link
Member

Choose a reason for hiding this comment

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

This validation should also be done on the server side, otherwise an attacker could bypass the client logic.

$invitationUrl = $this->urlHelper->linkToInvitationAbsolute($room);
$createMeetingParams->setModeratorOnlyMessage($this->l10n->t('To invite someone to the meeting, send them this link: %s', [$invitationUrl]));

if (!empty($room->logoutURL)) {
Copy link
Member

Choose a reason for hiding this comment

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

Should probably check if the logoutURL is empty. Anyway I would prefer to have the assignment at one place. Maybe

$createMeetingParams->setLogoutURL($room->logoutURL || $this->urlGenerator->getBaseUrl());

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.

Custom end meeting url

2 participants