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.

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

If a default expiration date is set, public shares cannot be created via OCS without an expiration date #10178

Open
Dagefoerde opened this issue Jul 10, 2018 · 7 comments
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: sharing

Comments

@Dagefoerde
Copy link
Member

The Moodle Nextcloud repository creates public shares via OCS in order to create links for use in Moodle. These links are expected to never expire, because you don't know for how long this link should be reachable from a Moodle course (usually for the duration of the term, but in some cases a course might last forever). However, we are unable to tell OCS that a share should last indefinitely: If we try, it always uses the default expiration date set by the administrator.

ShareAPIController treats empty dates like this:

if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}

Therefore we see no way to explicitly tell OCS to create a share with no expiration date if a default expiration date is set.

Steps to reproduce

  1. Set a default expiration date for public shares
  2. Create an OCS request for a public share, with an empty string for expirationDate ('')
  3. Check whether the created share expires after the default expiration date

Expected behaviour

If an empty string is passed no expiration date should be set for the given share.
We would expect something similar to updateShare where the expiration date is handled as follows:

if ($expireDate === '') {
$share->setExpirationDate(null);
} else if ($expireDate !== null) {
try {
$expireDate = $this->parseDate($expireDate);
} catch (\Exception $e) {
throw new OCSBadRequestException($e->getMessage(), $e);
}
$share->setExpirationDate($expireDate);
}

Actual behaviour

The default expiration date is used.

@nextcloud-bot nextcloud-bot added enhancement feature: sharing stale Ticket or PR with no recent activity bug and removed stale Ticket or PR with no recent activity labels Jul 10, 2018
@Dagefoerde
Copy link
Member Author

All the linked issues are unrelated (or at least: this is not a duplicate of them). Technically I consider this a bug, not an enhancement, because you can achieve this behaviour when UPDATING a share, but not when CREATING it. To me this looks like an edge case (explicitly no expiration intended) was missed during the implementation of createShare.

@rullzer rullzer self-assigned this Jul 10, 2018
@rullzer rullzer added this to the Nextcloud 14 milestone Jul 10, 2018
@nextcloud nextcloud deleted a comment from nextcloud-bot Jul 10, 2018
@rullzer
Copy link
Member

rullzer commented Jul 10, 2018

I'll look into this.

@rullzer
Copy link
Member

rullzer commented Jul 10, 2018

@Dagefoerde I see what you want to do.
Of course if the expiration date is enfroced this won't work 😉 but that is fine of course.

@rullzer
Copy link
Member

rullzer commented Jul 10, 2018

Ok so... the issue here is that we set the expiration date in the sharemanager. So if you set it to null (which it is by default) it will actually be set in the sharemanager on share creation

A possible solution would be

if explicitly an empty string is passed we set the date to some invalid expiration. If that is the case we just ignore the default expiration date. Unless it is enforced of course.

@Dagefoerde as a quick hack you could just try to remove the expiration date on the share you get back.

@Dagefoerde
Copy link
Member Author

@rullzer is the following sequence what you mean:

curl -i -d "path=/Nextcloud.mp4&shareType=3&publicUpload=false&permissions=1&expireDate=" -X POST -H "OCS-APIRequest: true" -H "Content-Type: application/x-www-form-urlencoded" "https://*****@****/nextcloud/ocs/v1.php/apps/files_sharing/api/v1/shares?format=xml"
curl -i -d "expireDate=" -X PUT -H "OCS-APIRequest: true" -H "Content-Type: application/x-www-form-urlencoded" "https://****@****/nextcloud/ocs/v1.php/apps/files_sharing/api/v1/shares/3?format=xml"

it works as described above, i.e. the invocation of createShare via the first command creates a share that expires after 7 days (my default expirationdate), even though expireDate is set; the invocation of updateShare on the share that was created by the first command removes the expiration date from that share.
As a quick hack this is fine, but it would be way better if expireDate would be handled at createShare already.

And I agree; if the expiration date is enforced this does not work (I get a 400 Expiration date is enforced). I guess this is fine, this would just be a very unfortunate setting in a system that is used in combination with Moodle.

@rullzer
Copy link
Member

rullzer commented Jul 12, 2018

@Dagefoerde yes that is what I mean.

Sure and I'll look into that. But since freeze is soon this probably won't be fixed in the upcomming releases

@MorrisJobke MorrisJobke removed the bug label Jul 24, 2018
@rullzer rullzer modified the milestones: Nextcloud 14, Nextcloud 15 Jul 31, 2018
@MorrisJobke MorrisJobke modified the milestones: Nextcloud 15, Nextcloud 16 Nov 7, 2018
@rullzer rullzer modified the milestones: Nextcloud 16, Nextcloud 17 Feb 26, 2019
@rullzer rullzer removed this from the Nextcloud 17.0.3 milestone Jan 29, 2020
@rullzer rullzer added this to the Nextcloud 17.0.4 milestone Jan 29, 2020
@rullzer rullzer modified the milestones: Nextcloud 17.0.4, Nextcloud 17.0.5 Mar 11, 2020
@rullzer rullzer modified the milestones: Nextcloud 17.0.5, Nextcloud 17.0.6 Mar 23, 2020
@skjnldsv skjnldsv added the 1. to develop Accepted and waiting to be taken care of label Apr 27, 2020
@skjnldsv skjnldsv removed this from the Nextcloud 17.0.7 milestone Apr 27, 2020
@jochenwezel
Copy link

Current status today at my Nextcloud Hub II 23.0.4: still not fixed

Creating shares with OCS (https://my.server/cloud/ocs/v1.php/apps/files_sharing/api/v1/shares?path=%2fshare-group-test.txt&format=xml) either

  • if default expiration date IS SET in admin UI: it always sets expiration default expiry date
    • regardless if I add an expiryData parameter or not
    • regardless to my transferred value
    • regardless to the "Enforce expiration date" checkbox
  • if default expiration date IS NOT SET in admin UI: it always sets EMPTY expiration date
    • regardless if I add an expiryData parameter or not
    • regardless to my transferred value

image

This issue has been tested with following environments:

  • NextCloud server: ERROR AS DESCRIBED ABOVE
  • OwnCloud server: WORKS CORRECTLY AS EXPECTED

Expected result

  • if default expiration date IS SET in admin UI AND "Enforce expiration date" checkbox IS CHECKED: it always sets expiration default expiry date
    • regardless if I add an expiryData parameter or not
    • regardless to my transferred value
  • if default expiration date IS SET in admin UI AND "Enforce expiration date" checkbox IS NOT CHECKED: it always sets expiration default expiry date
    • if I add an expiryData parameter with a value: to the parameter's value
    • if no expiryData parameter value is given: to the default value
  • if default expiration date IS NOT SET in admin UI: it always sets EMPTY expiration date
    • if I add an expiryData parameter with a value: to the parameter's value
    • if no expiryData parameter value is given: no expiration date is set up for the share

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: sharing
Projects
None yet
Development

No branches or pull requests

6 participants