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

storage: add group provider service and sharing SQL driver #1626

Merged
merged 6 commits into from
Feb 17, 2021

Conversation

ishank011
Copy link
Contributor

@ishank011 ishank011 commented Feb 11, 2021

Group sharing support in Reva. Fixes 100+ failures and once we add the correct LDAP group filters and fix this owncloud/core#38382, there should be a lot more.

@update-docs
Copy link

update-docs bot commented Feb 11, 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.

@ishank011 ishank011 force-pushed the group-provider branch 2 times, most recently from 1960466 to 873a4e8 Compare February 15, 2021 12:11
@ishank011 ishank011 marked this pull request as ready for review February 15, 2021 12:15
@ishank011
Copy link
Contributor Author

@butonic this is ready for review. It adds the missing config from reva and upgrades the reva and CS3APIs versions.

@ishank011
Copy link
Contributor Author

@butonic only the LDAP group filters need to be fixed:

// Group specific filters
// These might not work at the moment. Need to be fixed
&cli.StringFlag{
Name: "ldap-groupfilter",
Value: "(&(objectclass=posixGroup)(|(ownclouduuid={{.OpaqueId}})(cn={{.OpaqueId}})))",
Usage: "LDAP filter used when getting a group. The CS3 groupid properties {{.OpaqueId}} and {{.Idp}} are available.",
EnvVars: []string{"STORAGE_LDAP_GROUPFILTER"},
Destination: &cfg.Reva.LDAP.GroupFilter,
},
&cli.StringFlag{
Name: "ldap-groupattributefilter",
Value: "(&(objectclass=posixGroup)({{attr}}={{value}}))",
Usage: "LDAP filter used when searching for a group by claim/attribute. {{attr}} will be replaced with the attribute, {{value}} with the value.",
EnvVars: []string{"STORAGE_LDAP_GROUPATTRIBUTEFILTER"},
Destination: &cfg.Reva.LDAP.GroupAttributeFilter,
},
&cli.StringFlag{
Name: "ldap-groupfindfilter",
Value: "(&(objectclass=posixGroup)(|(cn={{query}}*)(displayname={{query}}*)(mail={{query}}*)))",
Usage: "LDAP filter used when searching for group recipients. {{query}} will be replaced with the search query",
EnvVars: []string{"STORAGE_LDAP_GROUPFINDFILTER"},
Destination: &cfg.Reva.LDAP.GroupFindFilter,
},
&cli.StringFlag{
Name: "ldap-groupmemberfilter",
// FIXME the storage implementation needs to use the members overlay to get the cn when it only has the uuid
Value: "(&(objectclass=posixAccount)(ownclouduuid={{.OpaqueId}}*))", // This filter will never work
Usage: "LDAP filter used when getting the members of a group. The CS3 groupid properties {{.OpaqueId}} and {{.Idp}} are available.",
EnvVars: []string{"STORAGE_LDAP_GROUPMEMBERFILTER"},
Destination: &cfg.Reva.LDAP.GroupMemberFilter,
},

@ishank011 ishank011 force-pushed the group-provider branch 4 times, most recently from 95c0eda to 2ae259f Compare February 16, 2021 08:42
Copy link
Member

@butonic butonic left a comment

Choose a reason for hiding this comment

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

cool, this brings us a lot closer. Could you move the sql and rest specific config flags to dedicated files? similar to the ldap.go.

things I noticed while testing:

  • requires wiping the share.json because the persisted data cannot be unmarshaled ... this is would be breaking change. since we are in tech preview I'm ok with this. but
    • please add a changelog.
  • searching for phy the physics-lovers group shows up and I can share with it but the 'shared with me' list when logged in as another user does not show them, so I cannot accept them.
  • the ldap filters need to be configured for the given ldap server.
    • we need to make sure glauth can handle them and maybe update the filters. ownclouduuid is understood by glauth

storage/pkg/flagset/users.go Outdated Show resolved Hide resolved
storage/pkg/flagset/groups.go Outdated Show resolved Hide resolved
storage/pkg/flagset/sharing.go Outdated Show resolved Hide resolved
@ishank011
Copy link
Contributor Author

ishank011 commented Feb 16, 2021

  • searching for phy the physics-lovers group shows up and I can share with it but the 'shared with me' list when logged in as another user does not show them, so I cannot accept them.

This means that the ldap-groupfindfilter and ldap-groupattributefilter are configured correctly for this deployment, but the ldap-usergroupfilter is not. It even says here that it'll never work since we're using the user's opaque ID to filter out groups; we need the memberof construct.

&cli.StringFlag{
Name: "ldap-usergroupfilter",
// 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
Value: "(&(objectclass=posixGroup)(ownclouduuid={{.OpaqueId}}*))", // This filter will never work
Usage: "LDAP filter used when getting the groups of a user. The CS3 userid properties {{.OpaqueId}} and {{.Idp}} are available.",
EnvVars: []string{"STORAGE_LDAP_USERGROUPFILTER"},
Destination: &cfg.Reva.LDAP.UserGroupFilter,
},

When a user logs in, we call the GetUserGroups method to get the list of groups and filter out the received shares based on this list. Since it doesn't work at the moment, the shares are not displayed for the recipient.

@ishank011 ishank011 force-pushed the group-provider branch 2 times, most recently from fadbb35 to 4c12cfb Compare February 16, 2021 13:37
@sonarcloud
Copy link

sonarcloud bot commented Feb 16, 2021

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information No Coverage information
No Duplication information No Duplication information

@butonic butonic mentioned this pull request Feb 17, 2021
16 tasks
@wkloucek wkloucek merged commit 888481a into owncloud:master Feb 17, 2021
ownclouders pushed a commit that referenced this pull request Feb 17, 2021
Merge: f416138 0784304
Author: Willy Kloucek <34452982+wkloucek@users.noreply.github.com>
Date:   Wed Feb 17 10:34:25 2021 +0100

    Merge pull request #1626 from ishank011/group-provider

    storage: add group provider service and sharing SQL driver
@ishank011 ishank011 deleted the group-provider branch February 17, 2021 10:18
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