When a user is deleted, is the uid record intentionally not deleted?
I notice that when a user is deleted, the uid record remains.
Is this because there is a bug in which the uid should be deleted but is not?
or is this intentional behaviour to prevent the uid from being recreated as a duplicate?
My testing code does large numbers of creations and deletions of users so the deleted uid records are building up.
On Feb 05, 2015, at 10:53 AM, Andrew Stuart wrote:
I notice that when a user is deleted, the uid record remains.
Is this because there is a bug in which the uid should be deleted but is not?
or is this intentional behaviour to prevent the uid from being recreated as a duplicate?
My testing code does large numbers of creations and deletions of users so the deleted uid records are building up.
I think it's mostly that this is an unexpected user case.
You've probably seen this, but the UniqueIDFactory behaves differently when the real system is running vs. when the test suite is running. In the latter case, we don't actually create new UID records in the database. Instead we use a file (protected by a lock) to monotonically increment a user-friendly UID. The testing machinery is hooked up to this so that when we reset the "database" between tests, the uid factory gets reset too.
Under normal execution, we create a uuid4() instance, which generates a random UUID, which is stored in the database, exactly so we can guarantee no duplicates (they are unlikely to happen, but we have to make sure).
What I didn't expect was for there to be a testing scenario which would hit the core while it was under normal operation mode, rather than testing mode. I think that's what you're doing, right?
If so, then indeed, the UID table never gets cleared, and the UID for deleted users never gets deleted.
I'm wary of deleting the UID when the user is deleted. It seems like this could leave us open to vulnerabilities where a UID gets reassigned to a new user. Your tests can't directly manipulate the database since you have to go through the REST API.
If this is really a problem for you, then we'll have to expose something in
the REST API for you to clear out the UID table, or at least specific UIDs.
What I could imagine is that, under some situations (e.g. the presence of an
environment variable perhaps?) a new top-level resource would get exposed
called <api>/testing and under that, would be resources you could touch
(DELETE?) that would reset bits of the database for you.
For example:
DELETE <api>/testing/uid_tablewould drop all UID rows. It would be up to you to call that at the appropriate time.
Thoughts?
Cheers, -Barry
Yes it’s a tough question. I agree with leaving the uid records in the database to prevent reuse. You are correct my test code is exercising a full working system. It would be good if my tests could leave the database in the same state after completion of each test.
The ideal would be a method that is exactly the same as the existing user delete code but also deletes the users uid record.
maybe as you suggest, url route names like:
DELETE <api>/testing/users/{user_identifier}' or DELETE <api>/reserved/users/{user_identifier}’
Personally I’d not bother with exposing or hiding reserved methods based on environment variables or anything else - as you say, it’s an internal API not mean to used by the public so developers should know not to be using reserved or testing methods in their production code. The extra functionality might introduce bugs and needs writing and testing and documenting - not worth it.
The fastest and simplest way ahead might be to duplicate the existing delete method under a new url route name (as above) and add “delete uid” functionality on top of the existing “delete user” code.
All of this is up to you though - I don’t want to be making work for others.
Just a heads up - there may be further similar questions coming up about the database because I’m only half way through writing my tests.
as
On 6 Feb 2015, at 3:04 pm, Barry Warsaw <barry@list.org> wrote:
On Feb 05, 2015, at 10:53 AM, Andrew Stuart wrote:
I notice that when a user is deleted, the uid record remains.
Is this because there is a bug in which the uid should be deleted but is not?
or is this intentional behaviour to prevent the uid from being recreated as a duplicate?
My testing code does large numbers of creations and deletions of users so the deleted uid records are building up.
I think it's mostly that this is an unexpected user case.
You've probably seen this, but the UniqueIDFactory behaves differently when the real system is running vs. when the test suite is running. In the latter case, we don't actually create new UID records in the database. Instead we use a file (protected by a lock) to monotonically increment a user-friendly UID. The testing machinery is hooked up to this so that when we reset the "database" between tests, the uid factory gets reset too.
Under normal execution, we create a uuid4() instance, which generates a random UUID, which is stored in the database, exactly so we can guarantee no duplicates (they are unlikely to happen, but we have to make sure).
What I didn't expect was for there to be a testing scenario which would hit the core while it was under normal operation mode, rather than testing mode. I think that's what you're doing, right?
If so, then indeed, the UID table never gets cleared, and the UID for deleted users never gets deleted.
I'm wary of deleting the UID when the user is deleted. It seems like this could leave us open to vulnerabilities where a UID gets reassigned to a new user. Your tests can't directly manipulate the database since you have to go through the REST API.
If this is really a problem for you, then we'll have to expose something in
the REST API for you to clear out the UID table, or at least specific UIDs.
What I could imagine is that, under some situations (e.g. the presence of an
environment variable perhaps?) a new top-level resource would get exposed
called <api>/testing and under that, would be resources you could touch
(DELETE?) that would reset bits of the database for you.
For example:
DELETE <api>/testing/uid_table
would drop all UID rows. It would be up to you to call that at the appropriate time.
Thoughts?
Cheers, -Barry
Mailman-Developers mailing list Mailman-Developers@python.org https://mail.python.org/mailman/listinfo/mailman-developers Mailman FAQ: http://wiki.list.org/x/AgA3 Searchable Archives: http://www.mail-archive.com/mailman-developers%40python.org/ Unsubscribe: https://mail.python.org/mailman/options/mailman-developers/andrew.stuart%40s...
Security Policy: http://wiki.list.org/x/QIA9
On Feb 06, 2015, at 04:17 PM, Andrew Stuart wrote:
Yes it’s a tough question. I agree with leaving the uid records in the database to prevent reuse. You are correct my test code is exercising a full working system. It would be good if my tests could leave the database in the same state after completion of each test.
The ideal would be a method that is exactly the same as the existing user delete code but also deletes the users uid record.
maybe as you suggest, url route names like: DELETE
<api>/testing/users/{user_identifier}' or DELETE<api>/reserved/users/{user_identifier}’
Thinking about this some more, and remembering that this is an admin interface (i.e. the authenticating proxy would *not* expose this), what about having just <api>/uids which could be used to access and delete uids. You couldn't post/patch to either the collection url (e.g. <api>/uids) or to individual uids (e.g. <api>/uids/7), but you could delete them *only if there is no user object associated with the uid*, otherwise you'd get an error.
This means you'd still have to delete the user and its uid in two operations,
but as this is primarily for testing purposes, the extra overhead should be
acceptable. We could even allow you to DELETE <api>/uids to clear out the
entire table (but again, only if there are no users...?). I put that
restriction in because I wouldn't want a bug in the proxy (or some other REST
API client) to break the whole system, which would definitely be possible if
you accidentally cleared out the uid table.
If it makes it easier to restrict proxy access to some url space, then we could put this under <api>/system or a new <api>/reserved resource.
Personally I’d not bother with exposing or hiding reserved methods based on environment variables or anything else - as you say, it’s an internal API not mean to used by the public so developers should know not to be using reserved or testing methods in their production code. The extra functionality might introduce bugs and needs writing and testing and documenting - not worth it.
Agreed.
Thoughts?
Cheers, -Barry
Sounds good to me.
On 10 Feb 2015, at 1:59 pm, Barry Warsaw <barry@list.org> wrote:
On Feb 06, 2015, at 04:17 PM, Andrew Stuart wrote:
Yes it’s a tough question. I agree with leaving the uid records in the database to prevent reuse. You are correct my test code is exercising a full working system. It would be good if my tests could leave the database in the same state after completion of each test.
The ideal would be a method that is exactly the same as the existing user delete code but also deletes the users uid record.
maybe as you suggest, url route names like: DELETE
<api>/testing/users/{user_identifier}' or DELETE<api>/reserved/users/{user_identifier}’
Thinking about this some more, and remembering that this is an admin interface (i.e. the authenticating proxy would *not* expose this), what about having just <api>/uids which could be used to access and delete uids. You couldn't post/patch to either the collection url (e.g. <api>/uids) or to individual uids (e.g. <api>/uids/7), but you could delete them *only if there is no user object associated with the uid*, otherwise you'd get an error.
This means you'd still have to delete the user and its uid in two operations,
but as this is primarily for testing purposes, the extra overhead should be
acceptable. We could even allow you to DELETE <api>/uids to clear out the
entire table (but again, only if there are no users...?). I put that
restriction in because I wouldn't want a bug in the proxy (or some other REST
API client) to break the whole system, which would definitely be possible if
you accidentally cleared out the uid table.
If it makes it easier to restrict proxy access to some url space, then we could put this under <api>/system or a new <api>/reserved resource.
Personally I’d not bother with exposing or hiding reserved methods based on environment variables or anything else - as you say, it’s an internal API not mean to used by the public so developers should know not to be using reserved or testing methods in their production code. The extra functionality might introduce bugs and needs writing and testing and documenting - not worth it.
Agreed.
Thoughts?
Cheers, -Barry
Mailman-Developers mailing list Mailman-Developers@python.org https://mail.python.org/mailman/listinfo/mailman-developers Mailman FAQ: http://wiki.list.org/x/AgA3 Searchable Archives: http://www.mail-archive.com/mailman-developers%40python.org/ Unsubscribe: https://mail.python.org/mailman/options/mailman-developers/andrew.stuart%40s...
Security Policy: http://wiki.list.org/x/QIA9
On Feb 10, 2015, at 02:04 PM, Andrew Stuart wrote:
Sounds good to me.
Another thought:
https://bugs.launchpad.net/mailman/+bug/1420083
Feel free to comment on the bug, instead of here.
Cheers, -Barry
participants (2)
-
Andrew Stuart -
Barry Warsaw