delete user home directory when deleting user

[C0rby]

No description provided.

[kulmann]

Hm... so this solves it for the OCS endpoint. But what about deleting a user directly in the accounts service? E.g. when we implement that in the accounts web UI? Or as a CLI command?

[C0rby]

Good point.
This ticket was about the ocs api (https://jira.owncloud.com/browse/OCIS-374) but yeah, when deleting the user in the accounts ui the home folder should probably be deleted too.

[phil-davis]

What happens about folders/files that are shared with other users, or as public links?
Will those be cleaned up (= deleted) when the delete of the home directory is being processed?

It would be nice to remove all (or as much as possible) "file metadata" of a user when the user is deleted.

[phil-davis]

If the user's home directory is correctly cleaned up when the user is deleted, then the tests will no longer need to be responsible to try and do that "under the hood". That is great.

So in this PR, it should be possible to remove the uses of DELETE_USER_DATA_CMD in .drone.star - that environment variable controls the command that is done by the tests after a user delete.

But at the moment the core test code still tries to do something "default" on OCIS if DELETE_USER_DATA_CMD is undefined. I think we will need to sort that out. So we might need to set the command to do something that does nothing persistent - e.g. set it to pwd or even to : (the "null" command in bash)

[butonic]

afaict there is some work to be done in reva to properly deprovision an account: cs3org/reva#1275

@phil-davis this PR is intended to allow the ocs api to deprovision accounts. wiping the users home will invalidate all share references and they will be filtered out if the referenced file no longer exists. not optimal because it accumulates shares. see the above ticket. good enough to deprovision accounts, I think.

I agree with @kulmann to move the deletion code to the accounts service ... for now ...

deprovisioning is a sensitive issue. it should happen in stages where the user is notified that his account will be frozen after n days, that his files will be deleted after 2n days, that his files will be moved to the archive after 3n days and that they will be truly deleted after 6n days ... or something like that, depending on company and legal policies. A hardcoded 'wipe now' like this PR is not the end goal.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
No Duplication information

[butonic]

@C0rby um this should go into the ocis/ocs because the proxy routes cloud/users requests to the ocis ocs service, not the reva ocs service ...

[C0rby]

@butonic, what do you mean? The code is in ocis/ocs or not?

[phil-davis]

@C0rby see PR #775 - the test suite does an under-the-hood delete of "user" files at the end of each scenario.
If I stop it doing that, then there are more test scenarios that fail. So I suspect that there are still things about an old user that are "remembered" when the next scenario creates a user of the same name again.

e.g. I suspect that versions and trashbin of the old user are still there - it would be nice to clear those out when the OCS Provisioning API DELETE is processed.

And maybe there are "remnants" of shares that cause side-effects in later scenarios.

[C0rby]

I'll have a look. 👀

[butonic]

@butonic, what do you mean? The code is in ocis/ocs or not?

nevermind, got confused...

[C0rby]

What the hell... for some reason it doesn't work anymore?
I have to investigate.... 🔬

[C0rby]

Ah, it does still work... For some reason the accounts are now created with the usernames not with uuids.

[C0rby]

Argh fork... The API tests fail... 😞

[C0rby]

Ah yes, of course... I updated reva in the wrong module.... Now let's see if it works.

[C0rby]

Phew... so the ocis storage api tests seem okay-ish now. Halfway there...

[phil-davis]

@C0rby I adjusted the way that some env variables work to control the core acceptance tests - owncloud/core#38077
That does not require any change to the env variables currently used in OCIS CI. It did impact CI in REVA cs3org/reva#1296

I bumped the core commit in OCIS, so that OCIS is running with those changes: #799 - I did that just to make sure that nothing was broken. That is the reason that you have a conflict in .drone.star, sorry!

[phil-davis]

Now I made core PR owncloud/core#38078 - that should be the little change that is needed in core to stop it from cleaning up user shares and files in the after-scenario. You should be able to test against that core branch do-not-cleanup-under-the-hood-on-ocis and get success if the Provisioning API user delete is doing the cleanup well.

[C0rby]

@phil-davis, no worries!

So I think I got the delete working for the OCIS storage (there is still one scenario failing though).
But I don't think the current approach of deleting the user folder using the CS3 delete method works for the owncloud storage.
The owncloud storage requires that the file needs to be in $STORAGE_DRIVER_OWNCLOUD_DATADIR/<userid>/files but when I call the delete method the path is $STORAGE_DRIVER_OWNCLOUD_DATADIR/<userid>

Meaning the storage checks if the path to delete has the prefix $STORAGE_DRIVER_OWNCLOUD_DATADIR/<userid>/files which in this case it doesn't.

[C0rby]

Opened a PR in the cs3apis repo to add a DeleteHome method to the storage provider.
cs3org/cs3apis#95

[kulmann]

cs3apis PR needs a rebase 😅

[refs]

I think I encountered the same problem that @C0rby did, why is the provisioning API using the user name attribute as ID? It results in an invalid token

[refs]

Ultimately minting a token relying on the provisioning API results in an invalid JWT attribute:

this is the contents of a working one:

[refs]

the previous error was caused because the IDP didn't match

[refs]

yup. Momentaneously hardcoding the IDP to https://localhost:9200 makes the testsuite happy following this approach. I have not yet dig into the new Spaces API.

[refs]

Ah, I see, the ocis fs changed from the time of this implementation took place, and it no longer works. Adjusting the IDP in the OCS service makes it work again, since the user equality checks is:

func isSameUserID(i *userpb.UserId, j *userpb.UserId) bool {
	switch {
	case i == nil, j == nil:
		return false
	case i.OpaqueId == j.OpaqueId && i.Idp == j.Idp:
		return true
	default:
		return false
	}
}

[C0rby]

I guess this can be closed since @refs did some work in another PR