cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Prachi Damle (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CLOUDSTACK-7073) Account/User creation: able to create user with the same name in the same domain in Clustered MS setup
Date Wed, 24 Dec 2014 18:55:13 GMT

    [ https://issues.apache.org/jira/browse/CLOUDSTACK-7073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14258444#comment-14258444
] 

Prachi Damle edited comment on CLOUDSTACK-7073 at 12/24/14 6:55 PM:
--------------------------------------------------------------------

>>>1) "So the databases prior to 4.5, which allowed the duplicate usernames will
fail to upgrade to 4.5 due to this schema change."

>>>We NEVER allowed duplicated usernames in the code, you can see the restriction
on that from the very beginning; if it happened, it was result of this very same bug "Account/User
creation: able to create user with the same name in the same domain in Clustered MS setup".
The correct fix for the bug you've fixed, would be removing the duplicated records, and then
making the upgrade.


Yes we have allowed DB to have duplicated usernames, since there is never a unique constraint
added on that field. From functionality perspective, we can delete a user and create another
with same username - so in the DB, two entries with same username exist.

So far, the code has been checking that there is not a user with same name already preventing
this usecase in normal scenarios. Except it breaks for the clustered management setup, so
we need some fix.


>>>2) Your other comment "Also, traditionally we have the 'removed' column in the
DB to mark a user as removed, and no unique constraint allowing us to add another user with
same name.". >>We should file a different bug for that - on removed event we should
simply NULL the userName field in cloud.users table. It should be done by overriding remove()
call in UserDaoImpl class.

>>>There is no other way to fix this bug but by introducing the domain_id field for
unique constraint check. We can't do it in the code as it won't work in cluster management
setup. We should put back the original fix, provide the customer with the steps to fix their
environments, and fix #2

We cannot just put back the original fix without having fixes in other areas:
a) Upgrade step is needed to take care of existing customer schemas. Without that step this
fix is not complete.
b) In the upgrade and in the API code  we need to decide on what to set the username field
to for the removed users, when we have to introduce these constraints. Simply setting it to
NULL is erasing the user history. Also this needs to be evaluated how it affects the usage
functionality. Will setting it simply to null affect any usage record collection?
c) There is another way to fix this using the DB constraints - by having another column say
'inactive' (boolean) tied up with the username and domainid. What this solves is the  limitation
of the 'removed' column not being useful since it can have a null value and MySQL unique constraint
wont work with null. Ofcourse, upgrade step is needed here for existing schemas.
d) Also this can be fixed through the code by having a global lock when we check if the username
is already existing during user creation. This will solve the issue with cluster management
setup, No?

So without these fixes we could not carry the original fix hence the revert for the 4.5 release.


was (Author: prachidamle):
>>>1) "So the databases prior to 4.5, which allowed the duplicate usernames will
fail to upgrade to 4.5 due to this schema change."

>>>We NEVER allowed duplicated usernames in the code, you can see the restriction
on that from the very beginning; if it happened, it was result of this very same bug "Account/User
creation: able to create user with the same name in the same domain in Clustered MS setup".
The correct fix for the bug you've fixed, would be removing the duplicated records, and then
making the upgrade.


Yes we have allowed DB to have duplicated usernames, since there is never a unique constraint
added on that field. From functionality perspective, we can delete a user and create another
with same username - so in the DB, two entries with same username exist.

So far, the code has been checking that there is not a user with same name already preventing
this usecase in normal scenarios. Except it breaks for the clustered management setup, so
we need some fix.


>>>2) Your other comment "Also, traditionally we have the 'removed' column in the
DB to mark a user as removed, and no unique constraint allowing us to add another user with
same name.". >>We should file a different bug for that - on removed event we should
simply NULL the userName field in cloud.users table. It should be done by overriding remove()
call in UserDaoImpl class.

>>>There is no other way to fix this bug but by introducing the domain_id field for
unique constraint check. We can't do it in the code as it won't work in cluster management
setup. We should put back the original fix, provide the customer with the steps to fix their
environments, and fix #2

We cannot just put back the original fix without having fixes in other areas:
a) Upgrade step is needed to take care of existing customer schemas. Without that step this
fix is not complete.
b) In the upgrade and in the API code  we need to decide on what to set the username field
to for the removed users, when we have to introduce these constraints. Simply setting it to
NULL is erasing the user history. Also this needs to be evaluated how it affects the usage
functionality. Will setting it simply to null affect any usage record collection?
c) There is another way to fix this using the DB constraints - by having another column say
'inactive' tied up with the username and domainid. What this solves is the  limitation of
the 'removed' column not being useful since it can have a null value and MySQL unique constraint
wont work with null. Ofcourse, upgrade step is needed here for existing schemas.
d) Also this can be fixed through the code by having a global lock when we check if the username
is already existing during user creation. This will solve the issue with cluster management
setup, No?

So without these fixes we could not carry the original fix hence the revert for the 4.5 release.

> Account/User creation: able to create user with the same name in the same domain in Clustered
MS setup
> ------------------------------------------------------------------------------------------------------
>
>                 Key: CLOUDSTACK-7073
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-7073
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the default.) 
>          Components: Management Server
>    Affects Versions: 4.5.0
>            Reporter: Alena Prokharchyk
>            Assignee: Alena Prokharchyk
>             Fix For: 4.6.0
>
>
> In the Java code we prohibit user to have duplicated names inside the same domain. But
in the DB the constraint is missing in cloud.account/cloud.user table, so it is still possible
to violate the rule by initiating the create call from parallel threads issued either by the
same MS, or by multiple MS in the clustered MS setup.
> To fix, have to introduce some kind of the global lock, or db constraint preventing multiple
threads to insert the record with the same username.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message