Database Migration using incorrect key for encryption

Issue Summary

I’m upgrading from Redash 6 -> 8. Looking at the instructions for the 7.0 upgrade it recommends setting up a REDASH_SECRET_KEY for database setting encryption. I did this, however after the upgrade started getting InvalidToken errors even though my secret key was set during the migration.

Looking at the code here: https://github.com/getredash/redash/blob/a34269cc7da334c58e9ad41e321d905f20faa4ec/migrations/versions/98af61feea92_add_encrypted_options_to_data_sources.py#L32

It seems during the migration it uses settings.SECRET_KEY (coming from REDASH_COOKIE_SECRET) rather than settings.DATASTORE_SECRET_KEY (which comes from REDASH_SECRET_KEY) so my config has been encrypted with the wrong key.

To work around this, I can set the REDASH_COOKIE_SECRET to match my REDASH_SECRET_KEY during migration, but feel this is pretty nasty upgrade issue.

Technical details:

  • Redash Version: 8.0.0.b32245
  • Browser/OS: Firefox / Linux
  • How did you install Redash: Docker

Related. Possibly useful.

Thank you for reporting this, @lloydw!

What happened here is that in v7, settings.SECRET_KEY was reading the value settings.DATABASE_SECRET_KEY is reading now. When we updated the code, we didn’t change the migration :grimacing:

This is another case where upgrading version by version (i.e. v6 -> v7 and then v7 -> v8) could help… maybe we should make it a recommendation in the upgrade docs.

1 Like

This is on my list. I’ll roll it in with the other updates this week.

Surely more importantly we should FIX this? There are still people using Redash 5 and earlier who might want to update in future - we have a framework in place so that it should be possible to upgrade across multiple versions and all the migrations will be applied in order. Lets use it!

I’ve just pushed two very simple PRs - one for master, one for the current release branch.


2 Likes

We added a recommendation to the upgrade docs to upgrade version by version, which will remedy this and other possible issues.

But considering you already fixed it, will surely merge it in and will create another release (8.0.1?) in the next days.

1 Like

Great! mind including:

… as well?

Definitely. I was keeping it open until I have time to cut a release :grimacing:

1 Like