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

default expire date config parameters can be set to 0 days #36569

Closed
phil-davis opened this issue Dec 11, 2019 · 5 comments · Fixed by #36829
Closed

default expire date config parameters can be set to 0 days #36569

phil-davis opened this issue Dec 11, 2019 · 5 comments · Fixed by #36829

Comments

@phil-davis
Copy link
Contributor

Steps to reproduce

  1. set shareapi_expire_after_n_days_user_share or shareapi_expire_after_n_days_group_share to 0
  2. send a sharing API request to create a share

Expected behaviour

The parameter is not allowed to be zero.

Actual behaviour

The share is created, and expires today

Server configuration

Current core with feature branch feature/user_group_share_expiration PR #36531

@phil-davis
Copy link
Contributor Author

Another thought is that the value 0 could be re-used to mean that there is no default expire date.
In that case, when sharing, the UI would display the expiry date field and leave it empty - so that users can pick an expiry date if they want, but will get no expiry date "by default"

@phil-davis
Copy link
Contributor Author

After discussion there is no need to prohibit zero days - if the admin wants new shares to expire "today" by default then so be it. It will work, and shares created today will expire around midnight. If an admin does not want that behaviour, then they do not have to set this parameter to 0.

@phil-davis
Copy link
Contributor Author

Currently a share expires at the very beginning of the expiry date - e.g. if I set to expire on 2019-12-20 then when I get to the office on 2019-12-20 the share has already expired and is gone.

That results in strange behaviour if a user creates a share and chooses today as the expiry date. Similarly if the admin sets the default expire days to 0. The share gets created with expiry date today. As soon as the share receiver accesses the share, the backend detects that the share has already expired and deletes the share. The share receiver never sees the share.

Also, IMO, it would make sense that if I specify 2019-12-20 as the share expiry date, then the share will keep working right up to the end of 2019-12-20.

See PR #36609 for how the code and tests could be adjusted.

@pmaier1 @micbar @HanaGemela what do you think?

@phil-davis phil-davis reopened this Dec 18, 2019
@phil-davis
Copy link
Contributor Author

https://github.com/owncloud/product/blob/master/modules/ROOT/examples/oc10/collaborators/expiration_date_for_collaborators.feature#L35 specifies

WHEN the specified day has just ended

So that matches the behaviour that I am suggesting above and in the PR.

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 27, 2020

The code to make the share expire at the end of the specified day was commit 8a1996c in #36573

After that code was done, it became sensible to allow "today" as the expiry date for a share. That was implemented in the sharing API (with some issues related to timezone offsets), but the webUI date-picker did not allow choosing "today".

Those things are fixed by PR #36829 - after that PR the expiry should all work nicely. Expiry can be from today upwards. Default expiry can be set to "today" (0 days) or greater, and can be enforced. The sharing API and webUI will respect those settings. Shares will expire at the end of the day specified.

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 a pull request may close this issue.

1 participant