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

Groups not created in drone tests #38382

Closed
ishank011 opened this issue Feb 8, 2021 · 9 comments
Closed

Groups not created in drone tests #38382

ishank011 opened this issue Feb 8, 2021 · 9 comments
Assignees

Comments

@ishank011
Copy link

ishank011 commented Feb 8, 2021

I'm working on adding group sharing to reva, for that we need to be able to retrieve groups from the ldap server using the userprovider service and the corresponding config. In a recent PR, the groups are added when authenticating as well: cs3org/reva#1456. But the logs show that no groups are actually retrieved

2021-02-05 13:25:36.001 INF drone/src/internal/grpc/services/authprovider/authprovider.go:114 > user id:<idp:"http://localhost:18000" opaque_id:"Alice" > username:"Alice" mail:"alice@example.org" display_name:"Alice Hansen" opaque:<map:<key:"gid" value:<decoder:"plain" value:"5000" > > map:<key:"uid" value:<decoder:"plain" value:"30000" > > > authenticated pid=15 pkg=rgrpc traceid=4789c1400ce382bb155dc43b7f9a2048

and they aren't added either https://drone.cernbox.cern.ch/cs3org/reva/498/10/2. Since in the acceptance tests, we try sharing with groups such as 'grp1', we need to add these for the CI to pass. @phil-davis @butonic

@ishank011
Copy link
Author

@phil-davis can you look at this please? We wanted to merge the group sharing PR so it'll be good to run the tests before that

@phil-davis
Copy link
Contributor

https://drone.cernbox.cern.ch/cs3org/reva/553/9/6

And group "grp1" has been created 
LDAP group exists: cn=grp1,ou=TestGroups,dc=owncloud,dc=com

That is logged by https://github.com/owncloud/core/pull/38392/files#diff-53ecaebb4552a8c12cb891ca32d9879923ec2d0226afeda7e559b9f631a1300cR3662

And that calls laminas-ldap getEntry() https://github.com/laminas/laminas-ldap/blob/2.12.x/src/Ldap.php#L1286 which claims that the group entry exists in the TestGroups ou.

@ishank011
Copy link
Author

Thanks for looking into this. Then I think the groupFilter needs to be fixed https://github.com/cs3org/reva/blob/master/tests/oc-integration-tests/drone/ldap-users.toml#L37

Because we're not able to retrieve the groups when logging in.

@phil-davis
Copy link
Contributor

I also notice that the ownCloud10 test code createLdapGroup() does:

		$this->ldap->add($newDN, $entry);
		\array_push($this->ldapCreatedGroups, $group);
		// For syncing the ldap groups
		$this->runOcc(['group:list']);

That last "group:list" is an oC10 command-line command which clearly does not exist in OCIS.

Is there any "magic" that I should do when running on OCIS to trigger OCIS to "sync/update" its view of which groups exist in LDAP?

@phil-davis
Copy link
Contributor

That last "group:list" is an oC10 command-line command which clearly does not exist in OCIS.'

Fixed in #38393 - that is just something that needed to be cleaned up, it won't fix this issue.

@ishank011
Copy link
Author

@phil-davis Do we execute a similar command for syncing users in oc10 as well? And do we have a parallel in OCIS? If not, how does it update the users list then?

@phil-davis
Copy link
Contributor

phil-davis commented Feb 12, 2021

When running on oC10 the code does occ user:sync after creating users if using LDAP. The only place that we do that combination nowadays is when testing the user_ldap app with oC10.

So nothing special seems to be done for new LDAP users when running the tests with OCIS. I guess that OCIS "just finds" the new user(s) in LDAP when the first API request arrives that mentions the user.

@ishank011
Copy link
Author

Okay, I think the groups should be synced as well then. We just need to update this filter.
groupfilter="(&(objectclass=posixGroup)(cn={{.OpaqueId}}))"

Note that the OpaqueId ID is that of the user, not of the group, so that's why it probably doesn't work. It says so here as well.

// FIXME the storage implementation needs to use the memberof overlay to get the cn when it only has the uuid,
// because the ldap schema either uses the dn or the member(of) attributes to establish membership

@ishank011
Copy link
Author

@phil-davis it works. I'll close the issue and create a separate one in OCIS about figuring out the group filter.

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

No branches or pull requests

2 participants