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

Changing username doesn't assign new owner to it's owned content items. #3492

Closed
Skrypt opened this issue Apr 20, 2019 · 21 comments
Closed

Changing username doesn't assign new owner to it's owned content items. #3492

Skrypt opened this issue Apr 20, 2019 · 21 comments
Milestone

Comments

@Skrypt
Copy link
Contributor

Skrypt commented Apr 20, 2019

Looking at this issue we should maybe think about adding an immutable User Id for marking content items owner instead of using it's username. Else, we will need to add something that will reassign content items to the new user username all the time. This doesn't look optimal for performance. Tell me which approach you guys prefer using and I will work on this.

@zl2fxy
Copy link

zl2fxy commented Apr 22, 2019

UserName will as a user property,Doesn't as User Key.It is good.
I think that we can create a user property like as user nickname,then UserName as key to store,and it's not to change.
The solution to this problem I encountered before。

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 22, 2019

The issue I found is that we store the UserName in the database in the Owner property of every document or ContentItemIndex row. But if someone requires to change it's UserName then the User account associated with these content items are going to be left as orphans.

UserName should be a User Account property (not mandatory). And Email should also be a User Account property (mandatory). But neither the email or username should be used as an identifier for a User Account. It should be an immutable Identifier like an int or a guid / sku.

@hishamco
Copy link
Member

I am not sure why there is ability to change the username while it represents the identity of that user?!!

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 22, 2019

It's not his identity it's just that we use it for the Owner everywhere. We could at least use the User Id that comes from the database but I think that this Id is not safe enough since it could be different from a staging server and a prod server. Right now, we can't import/export users because we could end up with mismatching User Id's.

The identity of the users right now, that I'm using, is the email since it's the only mandatory field that is unique per user. Though, we shouldn't use that as an identity either since users should be able to change their Account username or their email.

@hishamco
Copy link
Member

I think we shouldn't allow changing the username or email if we use it as identity

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 22, 2019

Changing a username or email on a user account should be pretty standard things to be able to achieve right?

@hishamco
Copy link
Member

No .. especially if the intend to use it as identity, you can checkout the other CMSs

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 23, 2019

Well, without looking. The idea is to have a separate identity guid to avoid needing to update all the Owner records when a UserName changes. Be standard or not. Should we keep the actual design or should we change it? Which other CMS's uses username as an identity?

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 23, 2019

No .. especially if the intend to use it as identity, you can checkout the other CMSs

To be honest this comment feels like trolling

@chinasqzl
Copy link
Contributor

In China, Email is not a common tool, Email must be filled in is not in line with China's national conditions.

@hishamco
Copy link
Member

@Skrypt introducing a guid is a good idea too, but my question why we need to change the username? Setting email as username could be optional like what I have seen in Umbraco

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 23, 2019

I think that neither of those 2 should be an identifier for a user account because they can change. A user could want to change it's email because it's no longer active. Though changing the UserName like in Github can have side effects that are not wanted.

https://help.github.com/en/articles/what-happens-when-i-change-my-username

GitHub cannot set up redirects for:

@mentions using your old username
Links to Gists with your old username

image

So using the UserName as an identifier can have side effects that are unwanted because if you allow people to change it then you need to run SQL Queries to alter all the records in a Database that has a reference to that UserName. And just like Github if we allow to change that UserName then we need to at least have these SQL Queries made automatically.

Though, if we'd be using a Guid that would not be necessary. The only thing we would need is to parse posts that have @Skrypt and replace it when stored to DB as @userGUID and when displaying that would just need to be replaced with the actual @UserName.

Setting Email as UserName for login purpose is an other topic. I think there's already a PR that has been created for that matter.

@hishamco
Copy link
Member

hishamco commented Apr 23, 2019

Sounds good, but did you think it's good idea to replace user guid with username everywhere!! What about the search indexing as example?

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 23, 2019

Lucene Indexes the ContentItemIndex table which has currently the Owner and Author columns which have a UserName in them. If we change these 2 we would need to resolve the current UserName from the UserIndex table each time we index a new document with Lucene (there is no issue there). The goal there would be to still be able to find documents by Owner or UserName.

Though changing a UserName would require at least that we reindex all the DB records owned by this user account. This could potentially take a lot of computing ressources since indexing with Lucene is quite slow. But that's the trade-off for having it working. I mean, this is probably what Github does somehow.

So the Idea would be to have a UserId column in UserIndex table simply.
And in the ContentItemIndex table you save the UserId in the Owner and Autor columns.
For each records in the Document table also you would need to save the UserId as the Owner in the JSON of the Content column.

An other idea would be to never be able to change a UserName and to only be able to change a Display Name. That could possibly work. But you should be also able to Login with the UserName or the Display Name or the Email. The Display Name here would somehow cloak the real user name. This solution could potentially cause also issues though when people are using @:UserName and @:DisplayName. And nothing prevents a user to change it's @:DisplayName more than once causing even more confusion.

The third option would be to never be able to change a UserName. Which would avoid all the confusion there is and would require no reindexing at all. Just allow to login with Email as a UserName.

@Skrypt
Copy link
Contributor Author

Skrypt commented Apr 23, 2019

I must add here that for me the UserName is not mandatory since the users that are loging on my website have extended User profiles which includes their "Name". They never need to use a UserName, only their email / password. This is mainly why I'm questionning the usage of that UserName as an identifier. Right now I'm making a link between that extended profile with the user account email. I could have used the user account UserName and that would have been the same mistake since we don't have any immutable UserId to make a link between a User and it's Extended Profile else than the DocumentId which can change from staging to prod if we import/export Users eventually.

If we never had the intent to add the import/export feature for security purpose I would undertand. Though for extending the user accounts I need to have an immutable UserId.

Example : On website # 1 I add user with UserName UserName1 and UserName2 in that Order. I get DocumentId = 1 for UserName1 and DocumentId = 2 for UserName2. But on website # 2 I do UserName2 before UserName1 ... I end up with mismatching Id's. So we are using right now UserName as an identifier because we needed something that is more reliable than DocumentId's.

So, I'd suggest that we instead generate a UserId with a MD5 on the first UserName used when creating the User and the creation date of that User. That way we will end up with true unique UserId's and we will be able to import/export user accounts even if user accounts have changed/swapped UserNames over time. This could also create issues with accounts created on both websites at different times...

@sebastienros
Copy link
Member

1- Isn't the username a user id already? In which case we might not be able to change it.
2- Should the username be seen as some friendly information like a display name "Jasmin Savard" and we should be able to change it. In which case we need an identifier that is not used for display purpose, but then add something else to be displayed.
3- Maybe we could keep the username as it is right now, and just create a new "Display name" property that the user could change? The username would be immutable, like the "id" in 2.

This is an "historical information" which we can't change. For instance each version has its own value, and we don't want to lose these. If a user is deleted, this value should also be kept even if there is no match. The solution is to have a custom feature that lets us reassign a specific username to another one. But it would be a manual operation, not automatic.

@sebastienros sebastienros added this to the backlog milestone Apr 25, 2019
@hishamco
Copy link
Member

I agree with @sebastienros, regarding deleting users I saw many scenarios out there some delete the user and keep it's username for all relared things like blogs and some remove the user and use Unknown instead

@jersiovic
Copy link
Contributor

jersiovic commented Dec 26, 2019

I agree with the option of using username as an unmutable identifier. And if it is needed to have an unique but mutable Display Name.
@Skrypt currently our users only provide the email, then we set an internal username that they are not aware of.

@Skrypt
Copy link
Contributor Author

Skrypt commented Dec 26, 2019

I made a PR for changing the Owner of content items when changing a UserName. But, I also, for my website did some handlers to reindex content items related with these Users. ASP.NET Identity framework allows changing a UserName so I think the proper solution would be to have a unique immutable "guid" like identifier for a User. Then change the Owner field to store that guid instead of the UserName. There is a lot of issues, right now, related by the fact we are using the UserName only.

@hishamco
Copy link
Member

+1 for introducing a unique user identifier

@Skrypt
Copy link
Contributor Author

Skrypt commented Dec 20, 2020

Closing as userId is now merged.

@Skrypt Skrypt closed this as completed Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants