SMB/CIFS mounts using logon credentials

[RobinMcCorkell]

In LDAP environments, users often have the same username and password to CIFS shares as they do to OwnCloud. It would be nice if this could be utilised to provide dynamic shares, that use the logon credentials, and also optionally use the username as the share name (in the case of Samba/Microsoft AD home directories).

I have some preliminary code that creates a new class inheriting off \OC\Files\Storage\SMB, however we could leverage the support brought through #7842 to merge them into \OC\Files\Storage\SMB. Ideas?

@icewind1991 @PVince81

[DeepDiver1975]

@blizzz

[RobinMcCorkell]

See Xenopathic/core@3b39dd30a1d9d52846c6897c803a7d0961eb900d

[blizzz]

This would mean that we need to save the credentials in plain text (or snake-oiled).

[RobinMcCorkell]

Well it is already implemented to a degree in iRODS, with the 'Use ownCloud login' configuration option. This just does the same thing, setting a session variable that is retrieved when the share is accessed.

[icewind1991]

A problem with using login credentials is that it breaks sharing, since we wont be able to mount the storage when another user is loggedin

[DeepDiver1975]

A problem with using login credentials is that it breaks sharing, since we wont be able to mount the storage when another user is loggedin

Indeed - in the context of iRODS this will be no issue as iRODS offers it's own sharing methanism which are planned to be implemented

[RobinMcCorkell]

The context of this change was to get home areas mounted dynamically without having the user's password hard coded into OwnCloud, such as in a domain environment. In such cases, is sharing even applicable? Would it be more work to turn it off entirely, or to store a cached copy when sharing is enabled for a file then serve that instead when accessed by others?

[PVince81]

@Xenopathic I suppose it might be possible to disable the "share" permission for all files on the SMB mount whenever the "use ownCloud login" option is set. This way the admin doesn't need to manually disable sharing globally.

[stephane-martin]

The clean way to do it would be that owncloud requests a kerberos ticket to
AD, using the user credentials, and then forgets about the creds. Or am i
wrong?
Le 21 mars 2014 15:40, "blizzz" notifications@github.com a écrit :

This would mean that we need to save the credentials in plain text (or
snake-oiled).

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/7843#issuecomment-38281459
.

[RobinMcCorkell]

Hmm, I'm not happy about the username retrieval. I noticed that the 'post-login' hook passes a 'uid' parameter equal to the UUID of the user, but for Samba mounts the username is required. This code is wrong in that it tries to get the display name of the user, which in my test environment was equal to the username but likely won't be in production. Is there any easy way to get a username from the UUID, or to get the username directly from the post-login hook parameters?

@PVince81 How can this be done? I checked the Share API reference, but it isn't clear on how to revoke PERMISSION_SHARE.

[PVince81]

For the user name instead of display name I don't know, sorry, would need digging the code.

@Xenopathic my idea for the permissions would be to override the isShareable() method in the SMB storage and always make it return false if that flag is set.
That method is called by getPermissions() in the Common storage class.

[RobinMcCorkell]

Just pushed some new commits up to my branch. Shame it isn't showing up automatically here, like a PR, but I'm apprehensive of pushing a PR until all the kinks have been sorted out. The commits fix the shareable problem, and fix the username problem.

Has anyone got any preference on whether this should be a separate class (as it is at the moment) or part of OC\Files\Storage\SMB? This approach avoids potential pitfalls, such as the shareable problem and any confusion with optional username and password.

[blizzz]

I would not mind adding it to the same class as long as it properly checks the conditions (LDAP yes/no) and correctly returns the isShareable(). It should also ship unit tests. Otherwise do a new one by sub-classing it. Also with unit tests as good as possible.

[RobinMcCorkell]

How should the configuration of it work? In the current implementation, the username and password are derived automatically, and there is a checkbox to use the username as the share name (aka as a home share). If the classes were merged, it could cause confusion as to the relevance of the username and password or the share name.

[PVince81]

Good point. I guess you could add a new backend "SMBLDAP" or something and make it extend the existing SMB storage class. That "SMBLDAP" (maybe you'll find a better name 😄) would provide its own config template in "files_external/config.php" where the username, password and share fields aren't set.