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

Fixes for /ocs/cloud/users when using the CS3 user backend #3096

Merged
merged 2 commits into from
Feb 3, 2022

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Feb 2, 2022

Description

This PR includes two workaround to get the CS3 backend for the ocs API to a somewhat working state. With this we'd no longer need to switch to the reva implemenation of ocs, e.g. when using an external LDAP server.

Related Issue

Motivation and Context

When using an external LDAP server we currently have to adjust the proxy configuration to point to the reva implemation of the ocs API. With this fix this should no longer be needed. I aware that these are mainly workarounds, but as we (at least to my understanding) agreed on removing the /ocs/cloud/users and ./groups services in the (not so long) run, I think they should be good enough. (btw, the /users/userid/groups implementation in reva also just returns an empty list, so we're not loosing anything here)

How Has This Been Tested?

manually using the WebUI / curl

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Up to now when using the CS3 backend (e.g. to use an external LDAP
server) queries to /users/{userid}/groups just errored out. This add a
simple stub to just return and empty group list for now.

This allows using and external LDAP server without having to fiddle with
the proxy configuration to redirect to the reva ocs implementation.
(Which also is just returning an empty group list currently)
@rhafer rhafer requested review from butonic, C0rby and wkloucek February 2, 2022 16:13
@update-docs
Copy link

update-docs bot commented Feb 2, 2022

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.

Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

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

Looks good. Just one nitpick.

return nil, err
}

roleIDs := make([]string, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
roleIDs := make([]string, 0)
roleIDs := make([]string, len(assignmentResponse.Assignments))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you meant make([]string,0, len(assignmentResponse.Assignments)) ? Without setting the initial length to zero the append will do funky things 😄.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you're right. :D

This we use reva to mint tokes for users when using the CS3 backend
(owncloud#2528) the user's roles are no
longer part of the token.

This adds a workaround to the RequireSelfOrAdmin middleware to Request
the user's role id on demand from the settings service.

Partial Fix for owncloud#2646
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@C0rby C0rby merged commit c4f1c1e into owncloud:master Feb 3, 2022
ownclouders pushed a commit that referenced this pull request Feb 3, 2022
Merge: 6cc5bfd ada93a9
Author: David Christofas <[email protected]>
Date:   Thu Feb 3 09:34:41 2022 +0100

    Merge pull request #3096 from rhafer/ocs-cs3

    Fixes for /ocs/cloud/users when using the CS3 user backend
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.

2 participants