urbanplatform / django-keycloak-auth

Middleware to allow authorization using Keycloak and Django for django-rest-framework (DRF). This package should only be used in projects starting from scratch, since it overrides the users' management.
MIT License
32 stars 14 forks source link

Fix UW Migration Issues #32

Closed diogosilva30 closed 2 years ago

diogosilva30 commented 2 years ago

When integrating this package with a pre-existing package a lot of migration issues would appear. I simply deleted all existing migrations, and made new ones.

When using this new version, no migration errors occur, and I can apply all migrations with a single py manage.py migrate

moritz89 commented 2 years ago

Hi, the problem with this change is that it basically breaks all existing apps that depend on the migrations. So if you have an app that implements a profile, that uses the 0005_auto_20211231_1702.py migration, this would break the migration chain for that app.

Therefore, I doubt that these changes will be acceptable.

diogosilva30 commented 2 years ago

Hi @moritz89, those are valid points. What I have tried:

  1. Created a sample django app

  2. Installed the default django_keycloak from this repo

  3. Create another app called "users" with the following model:

    class MyProfile(models.Model):
    user = models.OneToOneField(
        get_user_model(), on_delete=models.CASCADE
    )
    n = models.IntegerField()
  4. Run "python manage.py makemigrations". The resulting migration dependency is:

    dependencies = [
        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
    ]
  5. Nonetheless, I swapped the dependency for:

    dependencies = [
        ("django_keycloak", "0005_auto_20211231_1702"),
    ]
  6. Then applied migrations with python manage.py migrate. So far so good.

  7. Then, I swapped the first migration for this one. The migrations 2,3,4, 5 of keycloak I commented out the operations list, they are just passthrough.

  8. Tried to create another IntegerField() inside MyProfile, create and apply migrations are working.

    class MyProfile(models.Model):
    user = models.OneToOneField(
        "django_keycloak.KeycloakUserAutoId", on_delete=models.CASCADE
    )
    n = models.IntegerField()
    x = models.IntegerField() # --> new field, migration worked without any problem.
  9. For a final test, deleted the database, and re-applied all the migrations at once (python manage.py migrate). No issues, no need for two-step migration with keycloak.

From my tests, no problems on existing databases, and no problems in new databases. Feel free to do additional testing

moritz89 commented 2 years ago

One thing I've noticed is that I used the older AUTH_USER_MODEL = "django_keycloak.KeycloakUser" setting which does not run into this problem

moritz89 commented 2 years ago

I was able to recreate your error. A workaround is to first run the django_keycloak migration before running the general migration:

python manage.py migrate django_keycloak 0005_auto_20211231_1702
python manage.py migrate
python manage.py runserver
moritz89 commented 2 years ago

@diogosilva30 I'll close this issue as the proposed approach is unlikely to be merged. If you have other approaches, feel free to open a new MR.