nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.23k stars 4.05k forks source link

Allowed characters in username/userid #21313

Open Nils160988 opened 4 years ago

Nils160988 commented 4 years ago

How to use GitHub

Is your feature request related to a problem? Please describe. I'm always frustrated when a new bug arises from problems with some special characters (e.g. " " or "&") within userids. The manual says these characters are not allowed, but technically, it is not restricted and there are a lot of instances out there which have users containing spaces.

For one of my instances, I re-created all users without spaces and manually moved files, shares and other stuff to workaround different bugs (see below) which was rather painful. If this issue is solved, we could save admins and also (app) developers lots of time and prevent new issues.

Describe the solution you'd like

  1. Dis-allow " " and "&" (maybe even more potentially problematic characters) on user-creation.
  2. Migrate userids to new, "clean" ids. This will probably be the "do or die"-step of this issue. I know issues like https://github.com/nextcloud/server/issues/5488 have been closed, but seeing the trouble with userids, it might be worth to rethink the decision to close it. btw, that would be a nice enhancement also for other purposes.
  3. Deprecate userids with these characters and have a migration step which maps the ids to new ones.
  4. Possibly drop certain workarounds etc. which have been included to deal with special userids.

Describe alternatives you've considered Wait for more bugs related to this topic and fix them one by one.

Additional context Related issues (only open ones) I found in server (there are probably more in the apps' repositories) after a quick search: https://github.com/nextcloud/server/issues/21298 https://github.com/nextcloud/server/issues/17610 https://github.com/nextcloud/server/issues/16877 https://github.com/nextcloud/server/issues/15641

Mululu commented 4 years ago

@Nils160988 Thanks for your post!

We also have the problem!

Our IOS users cannot display their calendars on their devices because the link does not work due to the " "

Do you have a tip on how best to create users without too much effort? Every user has to reset his PW. Of course, they will not be happy.

RafalLukawiecki commented 4 years ago

I have just run into this issue. There was no warning when creating the user that spaces are disallowed or that they will cause future difficulties. As a result iOS calendar sharing is impossible now.

The documentation at https://docs.nextcloud.com/server/19/admin_manual/configuration_user/user_configuration.html does not mention that spaces are disallowed in usernames.

Is there a procedure for renaming a user? Deleting and recreating the user would mean moving their data, which would be hard as there are 100 GBs of media files shared by the user with others. I assume that deleting/recreating that user would break the sharing that is already present on a number of devices amongst other team members (perhaps I am wrong on that?)

Nils160988 commented 4 years ago

From your link:

Login names may contain letters (a-z, A-Z), numbers (0-9), dashes (-), underscores (_), periods (.) and at signs (@). After creating the user, you may fill in their Full Name if it is different than the login name, or leave it for the user to complete.

So no spaces allowed, but there is no warning, you are right with that.

The (quite manual, but I was able to script some steps) procedure for renaming was like: Create new users without spaces. Login once with the newly created users. Move files from old to new users. Recreate shares.

Depending on your usecases, this might be feasible for you or not. If you have groupfolders, it is quite easy, if you have many individual shares, it is nearly impossible. Manipulation of the database might be a way (test and always backup!). All devices have to be connected again because it is in fact a new user. That is why I asked for this feature :)

Edit: I forgot, @Mululu: For me, it worked to transfer the (hashed) passwords to the new accounts via db-commands. All new accounts could then login with the old passwords. Maybe, even clients might be able to connect automatically if they login with mail-address instead of username.

RafalLukawiecki commented 4 years ago

I have decided to go the painful way and I have recreated all the usernames—still waiting for everyone to reset their passwords etc. However, things work now.

I would say that there should be a clear documentation warning, in bold/emphasis, saying that usernames with spaces are not allowed but they will be accepted by Nextcloud. I also feel it is a bug if software does not perform sufficient input validation to match what, apparently, is in the specification of allowed usernames.

Nils160988 commented 4 years ago

Related: https://github.com/nextcloud/server/issues/22741

dertorsten commented 4 years ago

+1 for disallowing special characters in the first place. It's not only that nextcloud internal features are occasionally affected by this, but also 3rd party software where fixing or working around these issues is not an option. And - are there any reasons to have the email address not be unique?

skjnldsv commented 4 years ago

cc @rullzer @MorrisJobke @blizzz @nickvergessen

MorrisJobke commented 4 years ago

Is there a procedure for renaming a user? Deleting and recreating the user would mean moving their data, which would be hard as there are 100 GBs of media files shared by the user with others. I assume that deleting/recreating that user would break the sharing that is already present on a number of devices amongst other team members (perhaps I am wrong on that?)

There is a transfer ownership CLI command to do most of this.

MorrisJobke commented 4 years ago

I'm fine with disallowing those characters.

RafalLukawiecki commented 4 years ago

Is there a procedure for renaming a user? Deleting and recreating the user would mean moving their data, which would be hard as there are 100 GBs of media files shared by the user with others. I assume that deleting/recreating that user would break the sharing that is already present on a number of devices amongst other team members (perhaps I am wrong on that?)

There is a transfer ownership CLI command to do most of this.

I had a look, but it does not seem like the old sharing links would work after the change of ownership. I elected to recreate the users and to cause them discomfort than to live through months or years of broken link reports. In any case the disallowed characters should not be accepted as valid by user creation UI. Thank you for your help.

nickvergessen commented 4 years ago

https://github.com/nextcloud/server/blob/cbcf072b23970790065e0df4a43492e1affa4bf7/lib/private/User/Manager.php#L334-L337

We do check the character list. And well such accounts already exists and when it comes from LDAP it also needs to work.

So changing the list of (dis)allowed characters doesn't fix it anymore

pierreozoux commented 3 years ago

You have just spaces :) We have saml and usernames with é and other funky chars :)

It would be nice if this verification would also have been made on user creation, not just from Nextcloud user manager, but user creation in general (saml, ldap, external..)

blizzz commented 3 years ago

LDAP obliges to the constraint, but the SAML backend, for whatever reason, never did.

skjnldsv commented 3 years ago

Once it's decided, please make this available as an initial state ;)

robincafolla commented 3 years ago

Since #26935 was closed in favour of this I just want to re-iterate that usernames should ideally allow utf8 characters.

If you're allowing users to set something that is supposed to be representative of their name then you need to allow people to use the characters that actually make up that name (this is a touchy subject in many countries).

I don't know LDAP or SAML well but if those can't handle utf8 usernames then I'd strongly suggest following @Nils160988 suggestion of using a generated id for those services; or alternatively removing the ability for users to set user names entirely and randomly generating them (which amounts to the same thing).

solracsf commented 2 years ago

We just have faced this issue with a SAML user containing #. User is created, but it can't be added to any group, or view/add any files after login because of the # in the username.

pierreozoux commented 2 years ago

I actually have the same issue with groups.

In the case of groups, the UI doesn't let, but again, if you use saml.. For me it looks like a similar issue, but I'd also understand if you prefer me to open a new one. (I searched, but didn't find this issue already opened.)

hafiz-personal commented 2 years ago

I have also encounter this issue when creating a new user with username contains '&' character.

dweimer commented 2 years ago

This is still a problem, I was setting up a server using SAML with o365. Works fine for internal AD users that are synced with domain. However when I invite guest users into the o365 Azure domain from other domains and create a share they show as email_otherdomain.com#EXT#@mydomain.onmicrosoft.com. I can login, but accessing any shares creates a refresh loop on further checking I found its trying to load /remote.php/dav/files/email_otherdomain.com which returns a 404 file not found. So it appears that the # is stopping the remained of the name from appearing on the path.

Alwaysin commented 2 years ago

Not sure this is really related to this issue but I didn't want to create a new one. We always created our users with a displayname the same as the email adress, so with an '@'. It has always worked.

Since https://github.com/nextcloud/server/pull/33052 (more precisely https://github.com/nextcloud/server/pull/33052/commits/c0868f93f1175c32379f6e64b245b724c40478be), it looks like it is now throwing an error ("Invalid displayname"), but still creating the user, although not sending the welcome email.

alexx-250323 commented 1 year ago

It would be awesome if we could allow certain characters through the frontend admin page.

Piero3 commented 6 months ago

This issue affects "guest" users whose email address contains the "+" character. It seems to me that a valid email address should be accepted. And so the "+" character should be accepted in usernames.

LM-vb commented 5 months ago

Nextcloud 28.0.5

As stated above by @pierreozoux, this also affects group names. There is no warning about a group name not being allowed to contain spaces, creating such a group is hence possible. Adding users after creating the group works without problems.

However, when afterwards trying to share a folder with such a group, two things happen:

ykuksenko commented 4 months ago

I am not sure if my issue is an exact match for this, it seems like its essentially the same thing. Namely no issue creating the user but then the system fails when trying to perform some operation because of a character count (not invalid characters in this case).

I exported a user with php occ user:export bb192e0909a5f55e53c1ef1569fb0cd210509958ed843c97594aaeb215eb356e export/.

Upon import I get [ERROR] Invalid displayname the name came from keycloak with openid without any errors that I noticed.

The display name seems to be too long and cannot be edited in nextcloud UIs AFAICT.

The display name is `nc bb192e0909a5f55e53c1ef1569fb0cd210509958ed843c97594aaeb215eb356e`.
$ php occ user:list
  - bb192e0909a5f55e53c1ef1569fb0cd210509958ed843c97594aaeb215eb356e: nc bb192e0909a5f55e53c1ef1569fb0cd210509958ed843c97594aaeb215eb356e

Editing the Zip file and shortening the display name by hand gets past that error but fails with:

`ValueError: Invalid or uninitialized Zip object in /var/www/html/lib/private/Archive/ZIP.php:171` ```php Importing from ../export/bb192e0909a5f55e53c1ef1569fb0cd210509958ed843c97594aaeb215eb356e_2024-06-04_03-26-22.zip… An unhandled exception has been thrown: ValueError: Invalid or uninitialized Zip object in /var/www/html/lib/private/Archive/ZIP.php:171 Stack trace: #0 /var/www/html/lib/private/Archive/ZIP.php(171): ZipArchive->getFromName('migrator_versio...') #1 /var/www/html/custom_apps/user_migration/lib/ImportSource.php(56): OC\Archive\ZIP->getFile('migrator_versio...') #2 /var/www/html/custom_apps/user_migration/lib/ImportSource.php(148): OCA\UserMigration\ImportSource->getFileContents('migrator_versio...') #3 /var/www/html/custom_apps/user_migration/lib/ImportSource.php(160): OCA\UserMigration\ImportSource->getMigratorVersions() #4 /var/www/html/lib/public/UserMigration/TMigratorBasicVersionHandling.php(53): OCA\UserMigration\ImportSource->getMigratorVersion('usermigrationse...') #5 /var/www/html/custom_apps/user_migration/lib/Service/UserMigrationService.php(234): OCA\UserMigration\Service\UserMigrationService->canImport(Object(OCA\UserMigration\ImportSource)) #6 /var/www/html/custom_apps/user_migration/lib/Command/Import.php(87): OCA\UserMigration\Service\UserMigrationService->import(Object(OCA\UserMigration\ImportSource), Object(OC\User\User), Object(Symfony\Component\Console\Style\SymfonyStyl e)) #7 /var/www/html/3rdparty/symfony/console/Command/Command.php(298): OCA\UserMigration\Command\Import->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput)) #8 /var/www/html/3rdparty/symfony/console/Application.php(1040): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput)) #9 /var/www/html/3rdparty/symfony/console/Application.php(301): Symfony\Component\Console\Application->doRunCommand(Object(OCA\UserMigration\Command\Import), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Conso le\Output\ConsoleOutput)) #10 /var/www/html/3rdparty/symfony/console/Application.php(171): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput)) #11 /var/www/html/lib/private/Console/Application.php(213): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput)) #12 /var/www/html/console.php(113): OC\Console\Application->run() #13 /var/www/html/occ(11): require_once('/var/www/html/c...') ```

Unfortunately the freeipa backend deployment decided to die on me again so now I have a user I cannot migrate or access without rebuilding the IDM which I was hoping to drop entirely at this point due to stability issues.

pierreozoux commented 4 months ago

@ykuksenko you have another issue