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

Rework of PR#755 #1962

Merged
merged 19 commits into from
Apr 28, 2021
Merged

Rework of PR#755 #1962

merged 19 commits into from
Apr 28, 2021

Conversation

refs
Copy link
Member

@refs refs commented Apr 23, 2021

Use the CS3 API and Reva to deprovision users completely.

Two new environment variables introduced:

OCS_IDM_ADDRESS
OCS_STORAGE_USERS_DRIVER

OCS_IDM_ADDRESS is also an alias for OCIS_URL; allows the OCS service to mint jwt tokens for the authenticated user that will be read by the reva authentication middleware.

OCS_STORAGE_USERS_DRIVER determines how a user is deprovisioned. This kind of behavior is needed since every storage driver deals with deleting differently.

This is being tested with:

make test-acceptance-api \
TEST_SERVER_URL=https://localhost:9200 \
TEST_OCIS=true \
SKELETON_DIR=/Users/aunger/code/owncloud/core/apps/testing/data/apiSkeleton \
BEHAT_FEATURE=tests/acceptance/features/apiBugDemonstration/apiWebdavPreviews-previews.feature \
PATH_TO_CORE=/Users/aunger/code/owncloud/core

TODO:

  • Add PR description
  • Do we want to opt with this solution or instead use the StorageSpaces API?
  • Does this work with the owncloud driver? Does it have to?

@update-docs
Copy link

update-docs bot commented Apr 23, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@refs refs requested a review from wkloucek April 27, 2021 11:43
changelog/unreleased/ocs-user-deprovisioning.md Outdated Show resolved Hide resolved
@@ -358,6 +367,100 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) {
return
}

if o.config.RevaAddress != "" && o.config.StorageUsersDriver != "owncloud" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this not work for the owncloud driver?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@butonic https://github.com/cs3org/reva/blob/master/pkg/storage/fs/owncloud/owncloud.go#L1146-L1152

// If home is enabled, the relative home is always the empty string
func (fs *ocfs) GetHome(ctx context.Context) (string, error) {
	if !fs.c.EnableHome {
		return "", errtypes.NotSupported("ocfs: get home not supported")
	}
	return "", nil
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the logic probably should be more generic, but this at least makes the only consumer (CI) happy

ocs/pkg/service/v0/users.go Show resolved Hide resolved
Co-authored-by: Jörn Friedrich Dreyer <jfd@butonic.de>
.drone.star Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Apr 28, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

1.1% 1.1% Coverage
0.0% 0.0% Duplication

@refs refs requested a review from wkloucek April 28, 2021 11:32
@refs refs requested a review from butonic April 28, 2021 11:32
@refs refs merged commit 8ec196d into master Apr 28, 2021
@refs refs deleted the ocis-1798 branch April 28, 2021 11:33
ownclouders pushed a commit that referenced this pull request Apr 28, 2021
Merge: 215b015 1257bc3
Author: Alex Unger <6905948+refs@users.noreply.github.com>
Date:   Wed Apr 28 13:33:46 2021 +0200

    Merge pull request #1962 from owncloud/ocis-1798
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 this pull request may close these issues.

3 participants