Keyrock 8.0.0 database structure is incompatible with 7.9.X. Hash is mandatory

[jason-fox]

Related to https://github.com/ging/fiware-idm/issues/188

On upgrading Keyrock existing users cannot access the IDM. There is no help or documentation to help users to do so.

Usually the technique described in #188 is sufficient (although it would be better if a diff could be supplied in the docs, but 8.0.0 also brings a breaking change in the oauth_access_token table:

-  PRIMARY KEY (`access_token`),
-  UNIQUE KEY `access_token` (`access_token`),
+  `hash` char(64) NOT NULL,
+  PRIMARY KEY (`hash`),
+  UNIQUE KEY `oauth_access_token_hash_uk` (`hash`),

The hash column is now mandatory and holds data derived from the user's account. I wrote a simple script to work out the hashes, but ideally a proper sequelize function should be provided instead:

const crypto = require('crypto');

const token_ids = [
    'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
    'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
    'cccccccccccccccccccccccccccccccccccccccc',
    'd1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1',
    'd2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2',
    'm1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1',
    'm2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2'
];

token_ids.forEach(function(token_id) {
    const hash = crypto.createHash("sha3-256").update(token_id).digest('hex');
    console.log(token_id);
    console.log(hash);
    console.log('')
})

Which results in the following SQL:

 LOCK TABLES `oauth_access_token` WRITE;
 /*!40000 ALTER TABLE `oauth_access_token` DISABLE KEYS */;
 INSERT INTO `oauth_access_token` VALUES 
-('15682667caa4bb5ac15056fee3836b2980288bf2','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'8ca60ce9-32f9-42d6-a013-a19b3af0c13d','admin',NULL,NULL),
-('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa','alice',NULL,NULL),
-('bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb','bob',NULL,NULL),
-('cccccccccccccccccccccccccccccccccccccccc','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'cccccccc-cccc-cccc-cccc-cccccccccccc','charlie',NULL,NULL),
-('d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'d1d1d1d1-dddd-dddd-dddd-d1d1d1d1d1d1','detective1',NULL,NULL),
-('d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'d2d2d2d2-dddd-dddd-dddd-d2d2d2d2d2d2','detective2',NULL,NULL),
-('m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'m1m1m1m1-mmmm-mmmm-mmmm-m1m1m1m1m1m1','manager1',NULL,NULL),
-('m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'m2m2m2m2-mmmm-mmmm-mmmm-m2m2m2m2m2m2','manager2',NULL,NULL);
+('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa','alice',NULL,NULL, '12661599e24923dc17384a28644fbd2c0e30fa1cc7295772470d22729b054c8b'),
+('bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb','bob',NULL,NULL, '8d94b35f8eea7e1577e30fc75646dfeb4dd0982a083635028998d53ef590c7ec'),
+('cccccccccccccccccccccccccccccccccccccccc','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'cccccccc-cccc-cccc-cccc-cccccccccccc','charlie',NULL,NULL, 'f57858edab011913ac0a5d92f04987f4b34eab0d702c8198c1900871d7d87198'),
+('d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'d1d1d1d1-dddd-dddd-dddd-d1d1d1d1d1d1','detective1',NULL,NULL, '18a4605f12def28bbbbab7bbef23fe6e204d73432d9aee8514fc168037945221'),
+('d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'d2d2d2d2-dddd-dddd-dddd-d2d2d2d2d2d2','detective2',NULL,NULL, '1df5d6346470cc81d7a533f67a8399c052b5fc608b94972557138e10a335c5e1'),
+('m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1m1','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'m1m1m1m1-mmmm-mmmm-mmmm-m1m1m1m1m1m1','manager1',NULL,NULL, '853d6a374a92501e3e93d28184f9217941793ff646b636c04b35d20169c0d3b7'),
+('m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2m2','2016-07-30 12:14:21',NULL,NULL,NULL,NULL,'m2m2m2m2-mmmm-mmmm-mmmm-m2m2m2m2m2m2','manager2',NULL,NULL, '5603ade3a9d2303dbf3f28a35023a53c28297dc7db955784ac09b4c294ecae8b');

[apozohue10]

Hi @jason-fox

Do you mean editing the section in the readthedocs docu to include differences with the previous release and how should proceed to update release (e.g. running "npm run migrate_db")?

[jason-fox]

Precisely - this is just missing documentation.

[jason-fox]

Even better would be to keep a backup.sql file within the repo (generated by the seeding task) and users could use that to check the differences. In the case described above, it would be easier just to delete tokens of course, but it would be nice to be forewarned in the release notes or somewhere if/when database changes occur. The advantage of creating a file in the repo is that the history and diff are always correctly maintained.

[dwendland]

Wasn't the migration done automatically when the flag IDM_DB_SEED is set? At least I think it was doing it in the past. But when moving to 8.0.0 now, I had to run the "npm run migrate_db" manually.

[jason-fox]

IDM_DB_SEED is not set within the FIWARE Tutorials since they already have a pre-seeded database with users and OAuth tokens and applications and so on. The Tutorials are released as part of each FIWARE release and always use the latest FIWARE Components so it makes sense to pre-seed a Keyrock 8.0 tutorial with 8.0 database tables.

I think IDM_DB_SEED just seeds if the DB is not found anyway - it should be considered as a security flaw if the system could add a backdoor superuser with a known username and password to an already running system - this is why migration 7.X => 8.X should be detailed in the release note.

[aarranz]

I see this as a duplicated of #188. Database migrations are already shipped with KeyRock, so you are not required to do anything by hand regarding upgrading the database but executing the npm run migrate_db command. Documenting how to run this command in a safe way (e.g. doing backups) and so on ... would solve this issue.

Maybe, a new flag (disabled by default) for enabling automigration at container start can be useful for simple scenarios where just a single container of KeyRock is used connected to a single database and better if the instance not used for production. Something like:

1. IDM_DB_SEED is true and DB does not exist: => create database, do initial migration, seed database
2. else if IDM_DB_AUTOMIGRATE is true and DB exists: => migrate
3. else do nothing and try to use any existing database

Although migrations should not provide high risk of creating security problems: migrations are provided by the image (so they are official migrations and they are not programmed for changing the credentials of the super user); they can introduce security problems through bugs in the migration implementation and, also, they can provide some kind of data corruption. So I think they should be done using a maintenance procedure (database backup, running sanity check after applying the migrations, ...) that's the reason I don't recommend to enable them by default.

[apozohue10]

Thanks for the useful comments. I agree with you Jason that it makes sense to keep a backup file. Could one of the backup files from the tutorials be used? I will edit the documentation to include a section with changes on every release also. I will also take a look at PR 217 about the @aarranz suggestion.

[jason-fox]

I created this SQL from release 8.0: https://github.com/FIWARE/tutorials.Identity-Management/blob/master/mysql-data/backup.sql using the docker exec db-mysql /usr/bin/mysqldump -u root --password=secret idm > mine.sql technique - it should hold the data from the 8.0 seed.

Not much use on its own, but as the database changes over time it could prove more useful than deciphering all the incremental sequelize patch statements.

[jason-fox]

Stale