[For 10.4] Include expiration option for user and group shares

[phil-davis]

Description

This is a re-submit of PR #36324 and then PR #36531 with the commits squashed and rebased to the current master (that has "drop PHP 7.0" and phpunit7).

Please work on top of this in a separate branch for making acceptance tests or fixing bugs. Then we need to cherry-pick tests and bugs fixes into here in a controlled manner so that we do not write over each other's work.

Excerpt from the feature file:

• As a user I want to set an expiration date for collaborators in order to stay in control of access permissions by giving time-based access instead of granting access forever or until I manually remove the collaborator.
• As a user I want to automatically limit my responsibility for a guest account. Guests are bound to being a collaborator. If they are not a collaborator of any resource, they can't log in anymore.
• As an admin I want to set a default expiration date without enforcement to give a recommendation to users and help them maintain control over their resources. Still, I don't want to force or annoy users as some may have reasons to collaborate on resources for a longer or unrestricted amount of time, e.g., in projects.
• As an admin I have to follow corporate policies which dictate that data exchange between employees or also with externals may only take place for a certain maximum amount of time to prevent data leaks by human mistake or negligence.

Related Issue

• Fixes https://github.com/owncloud/enterprise/issues/2001
• Fixes Test: the fields of the last response should not include does not work with 'expiration' field #36545 [Acceptance tests related]

How Has This Been Tested?

CI acceptance tests added for:

• new capabilities added in capabilities API
• setting/clearing the new user and group expiration settings in the admin sharing settings UI page
• sharing with users and groups with/without expiration date on the UI
• using the sharing API to directly share/reshare/change share with default or specified expiry dates

Screenshots:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[phil-davis]

@individual-it acceptance tests are done. Please have a quick review (code should be the same as the previous PR) of test code.

[phil-davis]

@micbar @PVince81 please allocate whoever is appropriate to review the run-time code

[phil-davis]

Outstanding things to sort out:

• issue expireDate can be specified when the option for expireDate is not enabled #36568 - when these expireDate options are off, a client can still send API requests to create a share with an expire date. The API responds with success and has set an expire date. (this behaviour is OK - expireDate is now "always-on")
• issue default expire date config parameters can be set to 0 days #36569 - the default expire days can be set to zero 0. That results in shares getting created that expire today (I guess they will expire at midnight today). (this behaviour is OK - if the admin wants shares to expire "today" by default, then so be it)

[codecov]

Codecov Report

Merging #36573 into master will decrease coverage by 0.01%.
The diff coverage is 71.66%.

@@             Coverage Diff              @@
##             master   #36573      +/-   ##
============================================
- Coverage     64.68%   64.67%   -0.02%     
- Complexity    19071    19096      +25     
============================================
  Files          1268     1268              
  Lines         74522    74689     +167     
  Branches       1312     1320       +8     
============================================
+ Hits          48207    48307     +100     
- Misses        25930    25994      +64     
- Partials        385      388       +3

Flag	Coverage Δ	Complexity Δ
#javascript	54.12% <68.42%> (+0.06%)	0 <0> (ø)	⬇️
#phpunit	65.84% <72.67%> (-0.02%)	19096 <16> (+25)

Impacted Files	Coverage Δ	Complexity Δ
core/js/config.php	0% <0%> (ø)	0 <0> (ø)	⬇️
...ederatedfilesharing/lib/FederatedShareProvider.php	62.41% <0%> (-0.28%)	88 <1> (+1)
lib/private/Share20/ProviderFactory.php	85.36% <0%> (-6.74%)	17 <1> (+1)
...es_sharing/lib/Controller/Share20OcsController.php	93.73% <100%> (-0.03%)	206 <0> (ø)
settings/Panels/Admin/FileSharing.php	98.66% <100%> (+0.11%)	7 <0> (ø)	⬇️
core/js/shareconfigmodel.js	83.67% <100%> (+11.25%)	0 <0> (ø)	⬇️
settings/templates/panels/admin/filesharing.php	19.23% <20%> (+0.18%)	0 <0> (ø)	⬇️
core/js/sharedialogshareelistview.js	73.23% <40%> (-7.91%)	0 <0> (ø)
apps/files_sharing/lib/Capabilities.php	93.15% <71.42%> (-5.19%)	12 <1> (+2)
lib/private/Share20/DefaultShareProvider.php	97.9% <72.72%> (-0.46%)	120 <1> (+2)
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98fc511...f8f1631. Read the comment docs.

[micbar]

@phil-davis
We clarified the requirements here:

This feature should not disabled
The settins only have an influence on

• setting a default value
• enforcing it

The UI part will be always hidden in a dropdown.

Does that clarify it?

[jvillafanez]

Trying to clarify a bit the couple of issues:

issue #36568 - when these expireDate options are off, a client can still send API requests to create a share with an expire date. The API responds with success and has set an expire date.

There is no option to enable or disable the feature. The checkbox in the setting page is to set or not a default date and to enforce or not such default date as max date. If the checkbox is unchecked, it means that there is no default date set (or it should be ignored), but you can still set the expiration date normally.
From my point of view, it's fine that the clients can send API requests with any date, taking into account that the feature will be always enabled.

issue #36569 - the default expire days can be set to zero 0. That results in shares getting created that expire today (I guess they will expire at midnight today).

Personally, I don't think this is bad assuming it works. I also have some doubts about the current behaviour: it might expire at the beginning of the day, so it might be "expired on creation", but I don't think it will be much of a problem as long as the admin doesn't enforce it.
You can think that the admin, without enforcing a maximum date, is trying to force you to either set a different date or explicitly remove the expiration.

I agree we should probably control that scenario, but I don't think it's critical because if 0 is allowed I guess anyone can expect that the shares will expire today. It might not be useful, but as long as it doesn't break anything and the behaviour is clear I think we can live with that.

[phil-davis]

Re: 0 expire days and issue #36569 - see item 5 in #36550 where a decision was made that 0 is not allowed. (I am just pointing to where the decision was made, for information. IMO @jvillafanez analysis above is also valid)

So it would be possible to close #36569 and accept the current behavior

[phil-davis]

I have merged the changes from #36587 and squashed this PR. CI should be good. Final reviews are needed. @micbar and whoever should review the code here.
If anyone needs to investigate the history of the code development, then look at that PR #36587 and you can see all the commits that piled on each other.

[jvillafanez]

I'm missing an "event" variable here.

[PVince81]

seems this is likely not covered by tests, so might not even be working

[felixheidecke]

It works without passing event in. I guess it's already locally available. Added it for stringency.

[jvillafanez]

I haven't checked the acceptance tests.

[phil-davis]

@felixheidecke tests are passing, so I believe this works.
Can you respond to the review comments please?

[individual-it]

Most comments I made are about wording, specially because now the expiration date is not enabled/disabled but the default-expiration date can be set or not
Please apply the comments also to similar places in the rest of the feature files, I did not comment every single line

Additionally it would be good to also check if the share really was created or not, checking only the response might not be enough

[phil-davis]

@felixheidecke JS tests failing https://drone.owncloud.com/owncloud/core/22056/7/6

[phil-davis]

I cherry-picked the code from #36609 just now.
That adjusts the effective expiry of shares to be at the end of the day, not at the start of the day.

[individual-it]

Tests look good 👍

[phil-davis]

I just merged PR #36572 "Add share indicator in webUI"
That has some conflicts here - resolving that now...

Conflicts resolved, rebased, pushed. Now wait for another CI result....

[PVince81]

In general looks fine to me except for:

• the weird file with svg+suffix as extension
• the function that uses event but has none define which is likely broken

[phil-davis]

In general looks fine to me except for:

* the weird file with svg+suffix as extension

* the function that uses `event` but has none define which is likely broken

@felixheidecke or whoever is the dev who knows about this code, please respond to this and other outstanding comments.

e.g. there seem to be 4 review comments without responses/resolution here: #36573 (review)

I fixed the acceptance test comments and the 2 little code formatting comments.
Now there is just the 2 things mentioned here - "the weird file" and "the function that uses event"

[felixheidecke]

Aaaaand done!