Skip to content

Fix public calendars shared to circles - #23652

Merged
MorrisJobke merged 2 commits into
masterfrom
bug/circles-508/circles-public-shares
Nov 30, 2020
Merged

Fix public calendars shared to circles#23652
MorrisJobke merged 2 commits into
masterfrom
bug/circles-508/circles-public-shares

Conversation

@kesselb

@kesselbkesselb commented Oct 23, 2020

Copy link
Copy Markdown
Contributor

Fixnextcloud/circles#508

TypeError: Argument 2 passed to OCA\Circles\Db\CirclesRequest::getCircle() must be of the type string, null given, called in /var/www/html/apps/circles/lib/Service/CirclesService.php on line 271

It's not possible to fetch the principal from the circles app without a valid user session.

ca279f3 Update outdated phpdoc
ac8939a Fix

How to test:

  1. Create a calendar
  2. Create a link share for calendar
  3. Share calendar to a circle
  4. Open calendar link
  5. 💥

@kesselb

Copy link
Copy Markdown
ContributorAuthor

I don't like this approach. The knowledge if an app is able to handle a request without a user session should be in the app itself and not here. But the circles api defines that either a Circle object is return or an exception. Unfortunately we only catch CircleDoesNotExistException and QueryException so introducing another exception needs some time to reach all users.

@skjnldsv

Copy link
Copy Markdown
Member

Please rebase :)

@m4us1ne

Copy link
Copy Markdown

is there any progress about the review process?

as the QueryException is already catched inside circleToPrincipal.
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
The circles app depends on a valid userId. A public shared calender might be viewed by guests without a user session. For such requests the principal is null.
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb
kesselbforce-pushed the bug/circles-508/circles-public-shares branch from ac8939a to 35098a9CompareNovember 26, 2020 08:00
@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 26, 2020
@MorrisJobke
MorrisJobke merged commit 528516b into masterNov 30, 2020
@MorrisJobke
MorrisJobke deleted the bug/circles-508/circles-public-shares branch November 30, 2020 08:53
@MorrisJobke

Copy link
Copy Markdown
Member

/backport to stable20

@MorrisJobke

Copy link
Copy Markdown
Member

/backport to stable19

@MorrisJobke

Copy link
Copy Markdown
Member

/backport to stable18

@backportbot-nextcloud

Copy link
Copy Markdown

The backport to stable18 failed. Please do this backport manually.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sharing a Public Calendar with a Circle, breaks the public sharing.

6 participants

@kesselb@skjnldsv@m4us1ne@MorrisJobke@tcitworld@juliusknorr