justinmayer / kagi

WebAuthn security keys and TOTP multi-factor authentication for Django
BSD 2-Clause "Simplified" License
91 stars 10 forks source link

Add missing migration to repository #55

Closed evanottinger closed 1 year ago

evanottinger commented 1 year ago

When Kagi 0.3.0 is installed as a dependency in a Django application, the following migration is created:

(app) user:/app$ python manage.py makemigrations
Migrations for 'kagi':
  /home/user/.local/share/virtualenvs/app-4PlAip0Q/lib/python3.8/site-packages/kagi/migrations/0002_alter_backupcode_id_alter_totpdevice_id_and_more.py
    - Alter field id on backupcode
    - Alter field id on totpdevice
    - Alter field id on webauthnkey

This can cause problems if users extend Kagi models in their custom applications because Django will generate migrations for their custom apps dependent upon the newly generated Kagi migrations. However, since the Kagi migrations belong to a third-party package, they will not be committed to version control in the custom application's code repository. This will cause Django to throw an exception when dependent migrations are run against an application's database or even if manage.py makemigrations is run to attempt to create the new migrations at run-time.

This PR adds the missing migration to the Kagi code repository.

apollo13 commented 1 year ago

Hi @evanottinger,

this migration isn't exactly missing. It appears that you are running Django with DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField', this is why the migrations show a change for you. I guess the correct fix in kagi (for now) would be to add:

from django.apps import AppConfig

class KagiConfig(AppConfig):
    default_auto_field = 'django.db.models.AutoField'
    name = 'kagi'

and then maybe migrate to BigAutoField if wanted. What do you think @justinmayer ?

apollo13 commented 1 year ago

Closing this in favor of 61c1bce for now. Sorry for not merging the PR as it is currently.

evanottinger commented 1 year ago

Makes sense to me. Thank you!