Defect #5976
open
Uniqueness of User model fields is not checked sufficiently
0%
Description
Although, the User model defines :login and :mail as unique, it is not guaranteed that these are indeed unique in the database. Parallel requests can insert the exactly same users (apart from the id) if the transaction overlap.
Therefore I propose to add a unique-index to the database to ensure uniqueness. Also, I propose to always save the login in lowercase to properly use that index. There still has to be checked if that correctly works with the legacy mixed case logins (after r3807, r3813 because of #2473).
Updated by Daniel Felix almost 12 years ago
Well some kind of check is good, but if the database use some database driver which is transaction save, there shouldn't be some kind of duplicate.
Anyway, a check if the username/mail is available would be good.
Updated by Daniel Felix almost 12 years ago
- Category set to Accounts / authentication
Updated by Holger Just almost 12 years ago
Daniel Felix wrote:
Well some kind of check is good, but if the database use some database driver which is transaction save, there shouldn't be some kind of duplicate.
Well, no. Only the DBMS itself can ensure uniqueness constraints, at least with the transaction level typically used. And if there are no constraints defined, they will not be ensured naturally.
Updated by Etienne Massip almost 12 years ago
- Status changed from New to Confirmed
- Target version set to Candidate for next minor release
Indeed, the unique indexes are necessary (this is even documented in AR).
Updated by Etienne Massip almost 12 years ago
- Target version changed from Candidate for next minor release to Candidate for next major release
Updated by Daniel Felix almost 12 years ago
Holger Just wrote:
Well, no. Only the DBMS itself can ensure uniqueness constraints, at least with the transaction level typically used.
I know Holger, but I think this was some kind of misunderstanding.
With database driver I meaned things like InnoDB (transaction save) and MyIsam (not save, but faster), well it would be better if I called them database engines instead of drivers.
Well even a constraint in the db won't help if the database engine has no transaction support. ;-)
But anyway... beside this... I totally agree with you. This should be handled by some constraint, but there should be still some indicator if this username or mail is already in use before sending the registration to redmine. Just in case of the user friendliness.
Updated by Etienne Massip almost 12 years ago
Daniel Felix wrote:
Holger Just wrote:
Well, no. Only the DBMS itself can ensure uniqueness constraints, at least with the transaction level typically used.
I know Holger, but I think this was some kind of misunderstanding.
With database driver I meaned things like InnoDB (transaction save) and MyIsam (not save, but faster), well it would be better if I called them database engines instead of drivers.Well even a constraint in the db won't help if the database engine has no transaction support. ;-)
Please see #9685 and AR#validates_uniqueness_of.
My concern is more about the Users table storing Groups which have no mail...
Updated by Etienne Massip almost 12 years ago
Allowing a mail adress to be used for different users is often asked for so I don't think it would be a great idea to add a unique index here; about the login there's no MTI in RoR and neither sqlite, mysql or postgresql support unique indexes without considering null values.
So we could either have a table per class (will require more work in more code locations) or fill the login column with the name of the Group (Anonymous type has only one record).