NCSC-NL / taranis3

Taranis
Other
63 stars 19 forks source link

changing usernames #22

Closed bsi-lz closed 4 years ago

bsi-lz commented 5 years ago

We have the need to change user names. The column 'username' of the table 'users' is referenced in about 25 other tables (including our changes). The foreign key constraints in these tables are not set to 'ON UPDATE CASCADE'.

Is there a reason for this? Do you see any problems in changing these constraints to cascading update?

markov2 commented 5 years ago

I think that "ON DELETE CASCADE ON UPDATE CASCADE" would be on its place. Probably it is only missing because of laziness. I expect you find many more ways to add useful constraints to the database.

In general in SQL, it is not a smart idea to pick anything else that an 'id' sequence number to refer to. You should not refer via useful data, as there is a username.

On the moment, changing the username into something else is a bit more dangerous that you want... but not really hard to do in a script of a few lines:

If you want a change to the db scheme, I could prepare a fix for 3.7. You may also send me a patch with changes to install/db-load/taranis-schema.sql and a script in install/db-upgrade-scripts/ -- greetz, MarkOv


   Mark Overmeer MSc                                MARKOV Solutions
   Mark@Overmeer.net                          solutions@overmeer.net

http://Mark.Overmeer.net http://solutions.overmeer.net

markov2 commented 5 years ago

I have created internal issue 287, which contains a bunch of database scheme improvements. Probably, that issue will not be handled soon.

bsi-lz commented 5 years ago

I don't think ON DELETE CASCADE is a good idea. The current way to 'disable' users keep their actions in history. We would loose these informations if we drop them.

When i change the scheme i will provide a patch for you. Dunno when i will get to it though...

markov2 commented 5 years ago

That's a different abstraction. The task of the database scheme, is to maintain data consistency. The DB should not be hindered by logic on the higher levels: support user deletion even if the current application does not use that.

That's exactly why the requested 'ON UPDATE CASCASE' is not present: the scheme does not anticipated that it would ever happen. But I think that the scheme should have been made that flexible eventhough it would not be used. (The scheme should not be using the username as primary key in the first place)

When i change the scheme i will provide a patch for you. Dunno when i will get to it though...

I have reduced my involvement with NCSC to 1 week per month... so have limited time as well. -- greetz MarkOv