search

jazzband / django-oauth-toolkit

OAuth2 goodies for the Djangonauts!
https://django-oauth-toolkit.readthedocs.io
Other
3.13k stars 792 forks source link

It's impossible to revert migrations #1295

Closed yurasavin closed 12 months ago

yurasavin commented 1 year ago

Describe the bug I've faced with problem that it's impossible to revert migrations because migration 0006_alter_application_client_secret has no revert action

To Reproduce

  1. Apply migrations python manage.py migrate oauth2_provider
  2. Try to revert migrations python manage.py migrate oauth2_provider zero

Expected behavior Migrations are reverted

Version 2.2.0

Additional context

yurasavin commented 1 year ago

This issue is not a big deal so I've created a PR with fix https://github.com/jazzband/django-oauth-toolkit/pull/1296

I hope I haven't break any rules for the issues and PR's

n2ygk commented 1 year ago

Per #1093 (see CHANGELOG for 2.0.0 migration 0006 is not reversible because it performs a one-way hash of any cleartext secrets.

I'm not sure what the right answer is here.

yurasavin commented 1 year ago

Oh, I see now. OK we can't revert migrations without loosing the data, but how about to check in forwards_func if data has already hashed by prefix pbkdf2_sha256$ and skip hashing in case we reapply the migration?

n2ygk commented 1 year ago

That's already happening in https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/models.py

yura-savin commented 1 year ago

I mean in some cases you need to roll back migrations, I've faced this situation in my development. I don't care about is my secrets hashed or not in this case, I just need to roll back this migration from django migrations table, but without revert function it's become unpossible by applying manage.py migrate oauth2_provider 0005 for example.

n2ygk commented 1 year ago

Agreed. Maybe add the reverse migration to 0006 like you did in #1296 but have it print a warning or something.

dopry commented 12 months ago

This was resolved in https://github.com/jazzband/django-oauth-toolkit/pull/1296. However I would like to say that reverse migrations are not something we can guaranteed. I feel we should avoid allowing reversing where it would lead to a broken site. Best practice is to test migrations in UAT environments, and to back up production data before apply migrations to minimize data loss in case an unreversible migration is applied and someone must revert their code.

If someone were to reverse over migration 006 and revert the code, then auth would fail since client_id lookups would no longer work.