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

Shared links on SMB Share stop working if AD username is written different #18715

Closed
thechonta opened this issue Jan 7, 2020 · 16 comments
Closed
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap bug feature: ldap feature: sharing needs info stale Ticket or PR with no recent activity

Comments

@thechonta
Copy link

Steps to reproduce

  1. as an Administrator mount an SMB Store with in Databases saved credentials (taken form userlogin)
  2. login with AD/LDAP Account (surname.lastname)
  3. create an shared Link on an external Storage SMB
  4. Test the Link
  5. logout
  6. login with same account but write it like (Surname.Lastname or suRNAme.lASTnamE)
  7. try the shared Link (will ot work and the user will not see anny shares.
  8. logout
  9. login with the original writing of the account (surname.lastname)
  10. check the shared links and shares (will work)

Expected behaviour

Expected is, that the Nextcloud will treed anny writing of the ldapaccount like the AD so it will be allways the same User and wahtever the user did will be available no matter how the user was writing his loginname.

Actual behaviour

If an user createt shared links with his login written surname.lastname and login another day with an diferent writing of hi username, all shares will be stop working.
The lokal filesystem is not affectet and if the SMB Storage is mountet to the nextcloud with fixed credentials, also nothing will be afected.
Only if the credentials for the smb share are stored in the Database und was got on login the writing of the username matters.
Nextcloud need to learn that for Ldapaccounts it dosend matter how the username is written and treet everything the same.

In the Databese for every written username there will be created entrys in oc_storages so you have multiple lines for the same objekt and the same user.
oc_filecahce also affected.

Server configuration detail

Operating system: Linux 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64

Webserver: Apache/2.4.29 (Ubuntu) (apache2handler)

Database: pgsql PostgreSQL 10.10 (Ubuntu 10.10-0ubuntu0.18.04.1) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0, 64-bit

PHP version:

7.2.24-0ubuntu0.18.04.1
Modules loaded: Core, date, libxml, openssl, pcre, zlib, filter, hash, Reflection, SPL, sodium, session, standard, apache2handler, mysqlnd, PDO, xml, bz2, calendar, ctype, curl, dom, mbstring, fileinfo, ftp, gd, gettext, iconv, igbinary, imagick, intl, json, ldap, exif, mysqli, pdo_mysql, pdo_pgsql, pgsql, Phar, posix, readline, redis, shmop, SimpleXML, smbclient, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, wddx, xmlreader, xmlwriter, xsl, zip, libsmbclient, Zend OPcache

Nextcloud version: 17.0.2 - 17.0.2.1

Updated from an older Nextcloud/ownCloud or fresh install:

Where did you install Nextcloud from: unknown

Signing status

Array
(
)

List of activated apps
Enabled:
 - accessibility: 1.3.0
 - activity: 2.10.1
 - admin_audit: 1.7.0
 - cloud_federation_api: 1.0.0
 - comments: 1.7.0
 - dav: 1.13.0
 - federatedfilesharing: 1.7.0
 - federation: 1.7.0
 - files: 1.12.0
 - files_downloadactivity: 1.6.0
 - files_external: 1.8.0
 - files_markdown: 2.1.0
 - files_pdfviewer: 1.6.0
 - files_rightclick: 0.15.1
 - files_sharing: 1.9.0
 - files_trackdownloads: 1.6.0
 - files_trashbin: 1.7.0
 - files_versions: 1.10.0
 - files_videoplayer: 1.6.0
 - firstrunwizard: 2.6.0
 - gallery: 18.4.0
 - groupfolders: 5.0.5
 - issuetemplate: 0.6.0
 - logreader: 2.2.0
 - lookup_server_connector: 1.5.0
 - nextcloud_announcements: 1.6.0
 - notifications: 2.5.0
 - oauth2: 1.5.0
 - occweb: 0.0.4
 - password_policy: 1.7.0
 - privacy: 1.1.0
 - provisioning_api: 1.7.0
 - recommendations: 0.5.0
 - serverinfo: 1.7.0
 - sharebymail: 1.7.0
 - sharelisting: 0.2.0
 - spreed: 7.0.2
 - support: 1.0.1
 - survey_client: 1.5.0
 - systemtags: 1.7.0
 - text: 1.1.1
 - theming: 1.8.0
 - twofactor_backupcodes: 1.6.0
 - updatenotification: 1.7.0
 - user_ldap: 1.7.0
 - viewer: 1.2.0
 - workflowengine: 1.7.0
Disabled:
 - dashboard
 - encryption
 - passwords

Configuration (config/config.php)
{
    "instanceid": "***REMOVED SENSITIVE VALUE***",
    "passwordsalt": "***REMOVED SENSITIVE VALUE***",
    "secret": "***REMOVED SENSITIVE VALUE***",
    "trusted_domains": [
        "127.0.0.1",
        "nextcloud",
        "nextcloud.gehemim.de"
    ],
    "datadirectory": "***REMOVED SENSITIVE VALUE***",
    "skeletondirectory": "",
    "overwrite.cli.url": "https:\/\/nextcloud.geheim.de",
    "overwritehost": "nextcloud.geheim.de",
    "dbtype": "pgsql",
    "version": "17.0.2.1",
    "dbname": "***REMOVED SENSITIVE VALUE***",
    "dbhost": "***REMOVED SENSITIVE VALUE***",
    "dbport": "",
    "dbtableprefix": "oc_",
    "dbuser": "***REMOVED SENSITIVE VALUE***",
    "dbpassword": "***REMOVED SENSITIVE VALUE***",
    "installed": true,
    "memcache.distributed": "\\OC\\Memcache\\Redis",
    "memcache.local": "\\OC\\Memcache\\Redis",
    "memcache.locking": "\\OC\\Memcache\\Redis",
    "redis": {
        "host": "***REMOVED SENSITIVE VALUE***",
        "port": 6379
    },
    "ldapIgnoreNamingRules": false,
    "ldapProviderFactory": "\\OCA\\User_LDAP\\LDAPProviderFactory",
    "maintenance": false,
    "theme": "",
    "loglevel": 2,
    "app_install_overwrite": [
        "occweb",
        "sharelisting"
    ]
}

Are you using external storage, if yes which one: local/smb/sftp/...

Are you using encryption:

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...

LDAP configuration (delete this par if not used)

Client configuration

Browser: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Operating system: Ubuntu 18.04

Logs

Web server error log
Insert your web server log here 
Nextcloud log
Insert your Nextcloud log here
Browser log

Insert your browser log here, this could for example include:

a) The javascript console log
b) The network log
c) ...
@thechonta thechonta added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Jan 7, 2020
@kesselb
Copy link
Contributor

kesselb commented Jan 7, 2020

cc @nextcloud/ldap

@blizzz
Copy link
Member

blizzz commented Jan 8, 2020

@thechonta what happens with AD auth:

  1. We look up the DN according to the configuration and the provided login name. If a user was found we get the DN. The casing does not matter indeed.
  2. We bind against AD using the DN we got and the provided password

What happens with SMB auth:

  • we simply pass login name and password to the SMB service as we received it

You can get a value from LDAP that you can pass to an external storage configuration by specifying $home instead by setting the according field in LDAP's "Special attributes" in the Advanced tab. However, it does not work for provided credentials, only those pre-configured ones (and from the placeholder name you get that the initial idea was to use it in paths, but would work in any config field).

@thechonta
Copy link
Author

@blizzz
I cannt follow.
What is the solution that nextcloud will treed surname.lastname equal SURNAME.LASTNAME and do not create different logins/credentials/path for files on an SMB share.

For my understanding it would need an phaser that take the given writing of the username and than rewrite it from what ever to an standard structure for example low-case.
Only that way there is an guarantee that the username is correct and does not create multiple entry's for the file-path.

Kind regards

Chonta

@blizzz
Copy link
Member

blizzz commented Jan 8, 2020

What is the solution that nextcloud will treed surname.lastname equal SURNAME.LASTNAME and do not create different logins/credentials/path for files on an SMB share.

This is on the SMB server. Nextcloud won't tamper with the provided login name.

For my understanding it would need an phaser that take the given writing of the username and than rewrite it from what ever to an standard structure for example low-case.
Only that way there is an guarantee that the username is correct and does not create multiple entry's for the file-path.

I gave a hint on how you could use a value from LDAP to be used for your setup. Or configure AD that it would be matching comparisons on your login attributes case sensitively.

@kesselb
Copy link
Contributor

kesselb commented Jan 8, 2020

I think if SURNAME.LASTNAME and surname.lastname leads to two entries with the storage table hence all files are duplicate in filecache we should figure something out. However have not tested it myself.

@blizzz
Copy link
Member

blizzz commented Jan 8, 2020

@kesselb it's not the uid, just the login name passed to services.

@thechonta
Copy link
Author

thechonta commented Jan 8, 2020

What is the solution that nextcloud will treed surname.lastname equal SURNAME.LASTNAME and do not create different logins/credentials/path for files on an SMB share.

This is on the SMB server. Nextcloud won't tamper with the provided login name.

How? The SMB Server dont care how the username is written, the nextcloud Server does.
The access to files on the SMB Server will work no matter the username was written but shared links and favorites disapear because the Nextcloudserver handle it diferend in the Database.

For my understanding it would need an phaser that take the given writing of the username and than rewrite it from what ever to an standard structure for example low-case.
Only that way there is an guarantee that the username is correct and does not create multiple entry's for the file-path.

I gave a hint on how you could use a value from LDAP to be used for your setup. Or configure AD that it would be matching comparisons on your login attributes case sensitively.

Your hint i could not follow and to change the AD makes no sense for me.

in oc_storages for every used username you have this and it is both the same user.
Write the username diferent and you will have an new line.
The Problem, all links createt with the numericid 140 are not working, after a login with the username from id 150 was done.
|numeric_id| id | available|
|140 |smb::surname.lastname@SMB-Server//Freigabe//|1|
|158 |smb::SURNAME.LASTNAME@SMB-Server//Freigabe//|1|

In oc_filecache i have also for every numeric_id from oc_storage an entry.

The home:: in oc_storages exist only one time for each user.

Correct my if i am wrong, but the username handling the nextcloud uses in this cas is not optimal

@blizzz
Copy link
Member

blizzz commented Jan 9, 2020

For my understanding it would need an phaser that take the given writing of the username and than rewrite it from what ever to an standard structure for example low-case.
Only that way there is an guarantee that the username is correct and does not create multiple entry's for the file-path.

I gave a hint on how you could use a value from LDAP to be used for your setup. Or configure AD that it would be matching comparisons on your login attributes case sensitively.

Your hint i could not follow and to change the AD makes no sense for me.

in oc_storages for every used username you have this and it is both the same user.
Write the username diferent and you will have an new line.
The Problem, all links createt with the numericid 140 are not working, after a login with the username from id 150 was done.
|numeric_id| id | available|
|140 |smb::surname.lastname@SMB-Server//Freigabe//|1|
|158 |smb::SURNAME.LASTNAME@SMB-Server//Freigabe//|1|

In oc_filecache i have also for every numeric_id from oc_storage an entry.

The home:: in oc_storages exist only one time for each user.

Correct my if i am wrong, but the username handling the nextcloud uses in this cas is not optimal

Ah, i see. The external SMB storages creates the identifier out of the login name that was passed to it. Mh..... but this is not related to the LDAP backend.

Still, tampering with the provided credentials does not feel right. What works for you might break for others. @icewind1991 Do you have an idea, other than adding an option to read out the login name from a placeholder and only take the password from the login credentials?

@thechonta
Copy link
Author

thechonta commented Jan 9, 2020

It would be the easiest way if the uid from oc_accounts would be used for the connection.
The uid does not change, only if the ad account would be renamed because of name change of the user.

Real sure you cold only be with read out the SID from the user object from the AD.
If the username changes the SID will be the same and an algorithm nee to change for the SID to the new name wherever it is used.
But the best for the moment would be to use the given uid from accounts than i can write the user name as pleased and it will work.

@blizzz
Copy link
Member

blizzz commented Jan 9, 2020

The uid does not change, only if the ad account would be renamed because of name change of the user.

Negative, the userid in Nextcloud never changes, regardless what is done in AD. It is created initially and stays like that.

But the best for the moment would be to use the given uid from accounts than i can write the user name as pleased and it will work.

This might work for you in your specific setup, but not for others.

@kesselb
Copy link
Contributor

kesselb commented Jan 9, 2020

Looked into oc_credentials recently for another issue: #6343 (comment)

$qb->select('credentials')
->from(self::DB_TABLE)
->where($qb->expr()->eq('user', $qb->createNamedParameter($userId)))
->andWhere($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier)))

I think we are able to fetch the credentials in a case insensitive way. Not sure if possible for the storage table.

@blizzz
Copy link
Member

blizzz commented Jan 9, 2020

Looked into oc_credentials recently for another issue: #6343 (comment)

$qb->select('credentials')
->from(self::DB_TABLE)
->where($qb->expr()->eq('user', $qb->createNamedParameter($userId)))
->andWhere($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier)))

I think we are able to fetch the credentials in a case insensitive way. Not sure if possible for the storage table.

The userid, which is used in this query, is not necessarily the same as the login name. You cannot mix them up.

@kesselb
Copy link
Contributor

kesselb commented Jan 9, 2020

I can 🤣 You said the userid is always the same. So comparing lower(identifier) with strtolower($identifier) should work. But that's only about the credentials part and not having two record for the same credentials.

@blizzz
Copy link
Member

blizzz commented Jan 15, 2020

It is immutable. In opposite to the login name, which is. Also user id is not used for logging in and can be totally different from the login name (which is default behaviour with LDAP).

@szaimen
Copy link
Contributor

szaimen commented May 28, 2021

Is this Issue still valid? If not, please close this issue. Thanks! :)

@ghost
Copy link

ghost commented Jul 2, 2021

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

@ghost ghost added the stale Ticket or PR with no recent activity label Jul 2, 2021
@ghost ghost closed this as completed Jul 16, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap bug feature: ldap feature: sharing needs info stale Ticket or PR with no recent activity
Projects
None yet
Development

No branches or pull requests

4 participants