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

decomposedfs: make owner type optional #1978

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

butonic
Copy link
Contributor

@butonic butonic commented Aug 9, 2021

When reading the user from the extended attributes the user type might not be set, in this case we now return a user with an invalid type, which correctly reflects the state on disk.

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic butonic requested a review from labkode as a code owner August 9, 2021 11:25
@butonic butonic marked this pull request as draft August 9, 2021 11:46
@butonic
Copy link
Contributor Author

butonic commented Aug 9, 2021

This is the wrong approach:
IMO the user type must not be persisted in the storage, because it might change over time ...
only the id properties should be stored
for eos the user is always read from the gateway, based on the numeric id (and cached)

@butonic
Copy link
Contributor Author

butonic commented Aug 9, 2021

@ishank011 @labkode @aduffeck AFAICT the lightweight accounts are only needed for sharing, eg. when giving external users access to a file or folder. When moving the share folder logic to the sharestorageprovider by @aduffeck this will affect the existing storage drivers in two ways:

  1. the homeenabled flag becomes obsolete: all storage drivers will work in homeenabled=false mode. The other codepath can be eliminated, together with any special share folder handling in the drivers (the gateway still keeps some of it)
  2. the lightweight accounts checks can be moved into the share storage provider as well. It will be the only storage provider that guest accounts / lightweight users will have access to.

In the spaces concept a lightweight account will only have access to storage spaces that are in fact shared folders or files in other storage providers (in homeenabled=false) mode. Technically, they are jailed into a specific resource identified by a combined (id+relative path) reference.

Since guest acounts / lightweight accounts don't exist in the storage system (= not in LDAP) requests for them need to impersonate someone who does have access, eg. the share owner or the resource owner, but with an additional permissions mask. The storage driver itself does not need to handle lightweigth accounts. It should log them, but that can happen in the storage provider implementation.

@butonic butonic marked this pull request as ready for review August 9, 2021 13:02
@butonic
Copy link
Contributor Author

butonic commented Aug 9, 2021

This is ok for now, but we need to

  • replace the existing logic in the decomposedfs to fetch the user from the gateway, similar to eosfs

@ishank011
Copy link
Contributor

ishank011 commented Aug 9, 2021

the homeenabled flag becomes obsolete: all storage drivers will work in homeenabled=false mode. The other codepath can be eliminated, together with any special share folder handling in the drivers (the gateway still keeps some of it)

Why? Users will still make requests in their home namespace, and the storage drivers will determine the internal paths based on whether home is enabled or not. From that PR, just the shares folder will be moved out.

the lightweight accounts checks can be moved into the share storage provider as well. It will be the only storage provider that guest accounts / lightweight users will have access to.

Lw accounts won't have any space allocated to them, so they won't be able to create share references in the shares folder. So their requests would be ID and absolute path based.

Since guest acounts / lightweight accounts don't exist in the storage system (= not in LDAP) requests for them need to impersonate someone who does have access, eg. the share owner or the resource owner, but with an additional permissions mask.

This should depend on the storage drivers IMO. They'll have multiple resources shared with them by different users so it won't be possible for them to impersonate any particular user at a global level. For example, when listing shares in ocdav, we need to stat every resource. The only place where this can happen is at the storage level. The driver can say that it does not support such accounts if these need to be present in the auth provider.

@butonic
Copy link
Contributor Author

butonic commented Aug 9, 2021

the homeenabled flag becomes obsolete: all storage drivers will work in homeenabled=false mode. The other codepath can be eliminated, together with any special share folder handling in the drivers (the gateway still keeps some of it)

Why? Users will still make requests in their home namespace, and the storage drivers will determine the internal paths based on whether home is enabled or not. From that PR, just the shares folder will be moved out.

What is the difference beween a home namespace (/home) and a user namespaces (/users/einstein) when a share storage provider lists the shares a user has access to at /shares? IMO none: the gateway should return the user namespace of the currently logged in user (using the user layout) instead of hardcoding it to /home. /home does not need to exist. It is just an alias for the currenlty logged in user namespace.

the lightweight accounts checks can be moved into the share storage provider as well. It will be the only storage provider that guest accounts / lightweight users will have access to.

Lw accounts won't have any space allocated to them,

I agree

so they won't be able to create share references in the shares folder.

well, we do have a resharing permission in oc10 ... and I am not saying we need that

So their requests would be ID and absolute path based.

both? that does not make sense, either id only, path only or combined, right?

Since guest acounts / lightweight accounts don't exist in the storage system (= not in LDAP) requests for them need to impersonate someone who does have access, eg. the share owner or the resource owner, but with an additional permissions mask.

This should depend on the storage drivers IMO. They'll have multiple resources shared with them by different users so it won't be possible for them to impersonate any particular user at a global level. For example, when listing shares in ocdav, we need to stat every resource. The only place where this can happen is at the storage level. The driver can say that it does not support such accounts if these need to be present in the auth provider.

Right, for lightweight accounts the share storage provider will make requests to the actual storage provider, impersonating the share owner or the file owner. We should decouple that. Again, that moves the logic into the share storage provider ... and out of the drivers. The latter should be dumb and only optionally integrate with users that exist in the os/filesystem.

@ishank011 ishank011 merged commit 3fe79c8 into cs3org:master Aug 9, 2021
@butonic butonic deleted the owner-type-is-optional branch August 9, 2021 14:08
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