Ocfs lookup userid (update) #1052

PVince81 · 2020-08-04T13:35:00Z

Resend of #1033

also to be able to continue working to fix the issues, if they still exist.

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

PVince81 · 2020-08-04T14:07:42Z

the same tests fail on the reva repo, but they pass when I check out the whole of owncloud/ocis#409 and the connected deps.

to align with ocis-reva defaults, I've pushed a commit that changes the default user layout for OCFS to use the opaque id

@butonic FYI

if tests pass then it will confirm that it's the ocis-reva change that makes it work.

if changing the default in Reva is fine, we can keep it here

PVince81 · 2020-08-04T14:14:29Z

litmus tests on new dav failing now:

4. propfind_d0........... WARNING: response href for wrong resource

it is likely that this will also fail in owncloud/ocis#409 then, but there is no litmus run there apparently, not sure if we should add it on that level @individual-it

PVince81 · 2020-08-04T14:21:01Z

same failures as before. so there is more discrepancies in the setup

PVince81 · 2020-08-06T06:35:56Z

Litmus test runs fine locally with this branch and owncloud/ocis#409. Strange...

The user provider service now needs to be specified in the configurations

PVince81 · 2020-08-06T13:36:21Z

Pushed another missing setting

PVince81 · 2020-08-06T13:40:15Z

investigate whether we could go to the gateway instead

PVince81 · 2020-08-06T14:41:19Z

nice, the api tests pass.
next up: check that litmus failure

PVince81 · 2020-08-06T15:13:09Z

Right, the Litmus failure is because the test run is using "einstein" as user instead of the uuid.

If I run the test locally with "einstein" in the DAV URL I get the same failure.
But the following makes everything pass:

docker run -e LITMUS_URL="http://172.17.0.1:20080/remote.php/dav/files/4c510ada-c86b-4815-8820-42cdf82c3d51" -e LITMUS_USERNAME="einstein" -e LITMUS_PASSWORD="relativity" -e TESTS="props" owncloud/litmus:latest

now how to get this URL in Drone ?

it seems that UUID is hard-coded here https://github.com/owncloud/ocis-accounts/blob/8de0530f31ed5ffb0bbb7f7f3471f87f429cb2ea/pkg/service/v0/service.go#L45

so would be safe to use for now... I'll adjust Drone...

Fixed Drone config to use hard-coded UUID for einstein in the DAV URL.

refs · 2020-08-07T11:30:31Z

Rebasing #1064 onto this fixes the createShare test scenarios through the provisioning API /cc @individual-it. To test it:

# on this branch
git rebase fix-error-propagation

Replace on OCIS
run:

make test-acceptance-api TEST_SERVER_URL=https://localhost:9200 TEST_OCIS=true OCIS_REVA_DATA_ROOT=/var/tmp/reva/ SKELETON_DIR=apps/testing/data/apiSkeleton BEHAT_FILTER_TAGS='~@skipOnOcis&&~@skipOnOcis-OC-Storage' TESTING_REMOTE_SYSTEM=true BEHAT_FEATURE=tests/acceptance/features/apiShareManagementBasic/createShare.feature

I will address the comments on #1064

PVince81 · 2020-08-10T06:37:43Z

This PR here is ready for review @labkode @butonic @refs

butonic · 2020-08-10T09:19:45Z

drone/oc-integration-tests/storage-oc.toml

@@ -23,6 +23,7 @@ data_server_url = "http://localhost:11001/data"
 [grpc.services.storageprovider.drivers.owncloud]
 datadirectory = "/drone/src/tmp/reva/data"
 redis = "redis:6379"
+userprovidersvc = "localhost:18000"


Setting this will save a trip to the gateway. When left out it should fall back to the gatewaysvc in the [shared] section and work just as well.

the fallback did not seem to work, I had to add this

there was a strange thing where it was resolving some service, but that service did not implement the UserAPI, see #1033 (comment)

I didn't manage to find out what service it was resolving exactly

butonic · 2020-08-10T09:21:08Z

examples/oc-phoenix/storage-oc.toml

@@ -22,7 +22,8 @@ data_server_url = "http://localhost:11001/data"

 [grpc.services.storageprovider.drivers.owncloud]
 datadirectory = "/var/tmp/reva/data"
-
+redis = "redis:6379"
+userprovidersvc = "localhost:18000"


Setting this will save a trip to the gateway. When left out it should fall back to the gatewaysvc in the [shared] section and work just as well.

butonic · 2020-08-10T09:21:52Z

examples/oc-phoenix/storage-oc.toml

@@ -22,7 +22,8 @@ data_server_url = "http://localhost:11001/data"

 [grpc.services.storageprovider.drivers.owncloud]
 datadirectory = "/var/tmp/reva/data"
-
+redis = "redis:6379"


setting redis is optional. it defaults to localhost:6379

butonic · 2020-08-10T09:23:39Z

pkg/storage/fs/owncloud/owncloud.go

@@ -175,7 +179,7 @@ func (c *config) init(m map[string]interface{}) {
 		c.Redis = ":6379"
 	}
 	if c.UserLayout == "" {
-		c.UserLayout = "{{.Username}}"
+		c.UserLayout = "{{.Id.OpaqueId}}"


This changes the default, but will prevent username changes from breaking the storage lookup.

I put this there to align to the change in ocis-reva which also changes the default. I hope it's ok.
Otherwise we'll have further discrepancies.

butonic · 2020-08-10T09:24:25Z

pkg/storage/fs/owncloud/owncloud.go

+	Redis                string `mapstructure:"redis"`
+	EnableHome           bool   `mapstructure:"enable_home"`
+	Scan                 bool   `mapstructure:"scan"`
+	UserProviderEndpoint string `mapstructure:"userprovidersvc"`


We need the UserProviderEndpoint to look up a user by its username.

butonic · 2020-08-10T09:26:05Z

pkg/storage/fs/owncloud/owncloud.go

+		return nil, err
+	}
+	res, err := c.GetUser(ctx, &userpb.GetUserRequest{
+		UserId: &userpb.UserId{OpaqueId: usernameOrID},


When configuring the userprovider with ldap you need to configure a filter that checks both: the username attribute as well as the userid attribute.

butonic · 2020-08-10T09:27:40Z

pkg/storage/fs/owncloud/owncloud.go

@@ -881,6 +960,7 @@ func (fs *ocfs) ListGrants(ctx context.Context, ref *provider.Reference) (grants
 	grants = make([]*provider.Grant, 0, len(aces))
 	for i := range aces {
 		grantee := &provider.Grantee{
+			// TODO lookup uid from principal


The user lookup here needs to be implemented before the sharing tests can pass. at least I think they should fail somewhere without it.

butonic · 2020-08-10T09:31:33Z

@refs @PVince81 the TODO in the code for principal lookup is the only thing I think that needs to be implemented. I would expect the sharing tests to fail somewhere because of #1052 (comment)

PVince81 · 2020-08-10T09:55:00Z

@butonic all changes that were made here contributed to make CI pass. Maybe we are missing tests for triggering #1052 (comment) ?

butonic added 3 commits July 30, 2020 14:46

ocfs: lookup user to render template properly

8bbbd78

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

refactor, configurable addr

d4a46c5

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

use correct userprovider service

e8b185b

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

PVince81 requested a review from labkode as a code owner August 4, 2020 13:35

PVince81 mentioned this pull request Aug 4, 2020

ocfs: lookup user to render template properly #1033

Closed

OCFS: Change default user layout

ccc3220

ishank011 mentioned this pull request Aug 5, 2020

API tests fail when setting actual resource owner when creating share #1053

Closed

OCFS update configs for user provider svc

4b0bff3

The user provider service now needs to be specified in the configurations

Vincent Petry added 2 commits August 6, 2020 17:14

Fix Litmus test to use UUID in DAV URL

82f1bd3

Fixed Drone config to use hard-coded UUID for einstein in the DAV URL.

Adjust PR number in changelog

0acc7d9

PVince81 mentioned this pull request Aug 7, 2020

Download with signing key not working owncloud/ocis#436

Closed

refs mentioned this pull request Aug 7, 2020

Atomic requests to the accounts service owncloud/ocis-accounts#82

Merged

2 tasks

kulmann mentioned this pull request Aug 10, 2020

Update deps 2020 07 22 owncloud/ocis#409

Merged

63 tasks

PVince81 requested review from butonic and refs August 10, 2020 06:37

PVince81 mentioned this pull request Aug 10, 2020

[nomerge] Ocfs merge stray shares uid fixes #1068

Closed

butonic approved these changes Aug 10, 2020

View reviewed changes

labkode approved these changes Aug 10, 2020

View reviewed changes

labkode merged commit f6e4200 into cs3org:master Aug 10, 2020

butonic mentioned this pull request Aug 19, 2020

update reva to v1.1.1-0.20200819100654-dcbf0c8ea187 owncloud/ocis-reva#447

Merged

PVince81 mentioned this pull request Jan 18, 2021

Ability to specify user name in Webdav path despite UUID mode owncloud/ocis#1244

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ocfs lookup userid (update) #1052

Ocfs lookup userid (update) #1052

PVince81 commented Aug 4, 2020

PVince81 commented Aug 4, 2020

PVince81 commented Aug 4, 2020

PVince81 commented Aug 4, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

refs commented Aug 7, 2020 •

edited

Loading

PVince81 commented Aug 10, 2020

butonic Aug 10, 2020

PVince81 Aug 10, 2020

butonic Aug 10, 2020

butonic Aug 10, 2020

butonic Aug 10, 2020

PVince81 Aug 10, 2020

butonic Aug 10, 2020

butonic Aug 10, 2020

butonic Aug 10, 2020

butonic commented Aug 10, 2020

PVince81 commented Aug 10, 2020

Ocfs lookup userid (update) #1052

Ocfs lookup userid (update) #1052

Conversation

PVince81 commented Aug 4, 2020

PVince81 commented Aug 4, 2020

PVince81 commented Aug 4, 2020

PVince81 commented Aug 4, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

PVince81 commented Aug 6, 2020

refs commented Aug 7, 2020 • edited Loading

PVince81 commented Aug 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

butonic commented Aug 10, 2020

PVince81 commented Aug 10, 2020

refs commented Aug 7, 2020 •

edited

Loading