Orchardcore: User accounts improvements

Created on 30 Jan 2020  路  4Comments  路  Source: OrchardCMS/OrchardCore

I'd like to revive this one:

Right now, is it possible to create a new account with an email that already exists in the database?

  1. I think we should support being able to log with a username or an email, and it might just be the only option
  2. I don't think we should support being able to log with only the email or only the username (i.e. prevent the other one) as long as we don't expose if the entry exists in the db when the login fails, to prevent lookups
  3. Unicity:
    usernames should be unique (and case insensitive to prevent case spoofing)
    emails should be unique (and case insensitive to prevent case spoofing)
  4. Usernames and Emails should be unique, as a whole, such that an existing email can't be used as a new username
  5. Like this PR we should be able to enforce the email to be valid before enabling an account, as a setting (in the admin)
  6. Usernames should not be changed if they are used to represent a user reference (they are). Or we should introduce a new identity for users and use it, and also store usernames for historical data, not reference integrity (like we need to for tenants)
  7. Emails can be changed as they are not used as user identity (and they shouldn't, because they can be changed)
  8. I don't see why the username and the email should not have the same value, even if they can be changed individually, as long as the value doesn't collide with another existing user. There should then be no validation on the username, but validate the email.
P1 Users

Most helpful comment

The identifier of a user account should not be editable

  1. For security reasons, as if an account is deleted, another account could use a previous identifier to impersonate technical permissions.
  2. For data integrity as an identifier is used for referencing to an account, and that would break any reference (e.g. link to user, owner property in content item)
  3. For idempotency of import/export such that two systems share a logical identifier that is used to migrate data. Hence an auto-increment integer like the document id should not be used as a unique identifier.

The identifier of a user account should be random, and not entered by a user

To prevent accounts from being impersonated in case an identifier was leaked. So even at the time of the creation of an account, the identifier should be computer generated, or have a part that is computer generated.
The identifier should be normalized such that its usage is case insensitive, and any attempt to use a different case will match an existing account. For instance all values should be stored and looked up with a defined char case.

Implications

  • We can't use the UserName as an identifier, as it is provided by user input
  • We need to create a new property to store the identifier
  • We can use the existing usernames as identifiers for existing accounts provided we can't edit identifier anymore. That will prevent a data migration.
  • We need to understand what User.Identity.Name represents and how it's used relative to the username property, which was used an an identifier as of now.
  • We should not use the document id anymore but the new identifier.

All 4 comments

If username can be changed, we need to apply the changes to the content items that have it as owner.

I think what @sebastienros means is that the Owner should be kept for history but we should also have an immutable ID to identify the real Owner instead. That way, no need to change the Owner when the username is changed. This also means that permissions should be then checked against that new immutable ID.

The identifier of a user account should not be editable

  1. For security reasons, as if an account is deleted, another account could use a previous identifier to impersonate technical permissions.
  2. For data integrity as an identifier is used for referencing to an account, and that would break any reference (e.g. link to user, owner property in content item)
  3. For idempotency of import/export such that two systems share a logical identifier that is used to migrate data. Hence an auto-increment integer like the document id should not be used as a unique identifier.

The identifier of a user account should be random, and not entered by a user

To prevent accounts from being impersonated in case an identifier was leaked. So even at the time of the creation of an account, the identifier should be computer generated, or have a part that is computer generated.
The identifier should be normalized such that its usage is case insensitive, and any attempt to use a different case will match an existing account. For instance all values should be stored and looked up with a defined char case.

Implications

  • We can't use the UserName as an identifier, as it is provided by user input
  • We need to create a new property to store the identifier
  • We can use the existing usernames as identifiers for existing accounts provided we can't edit identifier anymore. That will prevent a data migration.
  • We need to understand what User.Identity.Name represents and how it's used relative to the username property, which was used an an identifier as of now.
  • We should not use the document id anymore but the new identifier.
Was this page helpful?
0 / 5 - 0 ratings

Related issues

szilardcsere89 picture szilardcsere89  路  3Comments

jeffolmstead picture jeffolmstead  路  4Comments

aghili371 picture aghili371  路  3Comments

ns8482e picture ns8482e  路  4Comments

randaratceridian picture randaratceridian  路  3Comments