Swappable models do not swap as expected

[jmichalicek]

I have updated the title since this turns out to be more than just an issue related to incompatible primary keys when swapping, but a general failure which really seems to be related to Django not fully supporting swappable models outside of the user model yet.

I was testing using a custom credential model set with settings.OTP_WEBAUTHN_CREDENTIAL_MODEL to use a UUIDField() for the primary key. I found that when I do this, even if also providing a custom attestation model which points directly at my custom credential model, the django_otp_webauthn.0003_alter_webauthnattestation_credential migration fails with the following error:

  Applying django_otp_webauthn.0003_alter_webauthnattestation_credential...Traceback (most recent call last):
  File "/app/.venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 103, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/.venv/lib/python3.12/site-packages/psycopg/cursor.py", line 97, in execute
    raise ex.with_traceback(None)
psycopg.errors.CannotCoerce: cannot cast type bigint to uuid
LINE 1: ...COLUMN "credential_id" TYPE uuid USING "credential_id"::uuid

I believe I have a fix for this by updating the 0001 migration and adding swappable option to the Meta class for each model. The option is not documented as far as I have found so far. I had to go look at the django.contrib.auth models and migrations. I want to do a couple more tests over the weekend and then should have a PR up for the fix.

[jmichalicek]

Just a quick update that the further testing is still on my todo list. It's been a busy weekend and following week.

[jmichalicek]

Alright, I'm super slow... it's been busy at work. My hope is to confirm a few things tomorrow and then get a PR in.

[jmichalicek]

Well, this gets more and more interesting. Sorry for the novel. The tldr; is that a 3rd party app probably needs pulled in to do this. The django docs on migrations make specific mentions of swappable models and even functions to use in the migrations for resolving them, but there is no other documentation.

I dug through the django.contrib.auth code to see how the user model is set up and found an undocumented meta option, swappable and started playing with this and updated migrations existing migrations to handle the swappableness(I just made that word up) because just making new ones didn't work.

For example

class WebAuthnCredential(AbstractWebAuthnCredential):
    """A OTP device that validates against a user's credential.

    See https://www.w3.org/TR/webauthn-3/ for more information about the FIDO 2
    Web Authentication standard.
    """

    class Meta(AbstractWebAuthnCredential.Meta):
        swappable = "OTP_WEBAUTHN_CREDENTIAL_MODEL"

I was able to get things "almost" working. I was able to swap out both the credentials and attestation models and just the credential model (although I suspect a few bugs may have still existed in migrations such as 0003_webauthncredential_hash_binary_to_hex since it uses AddField and references a specific model, etc). When only swapping out the attestation model I ran into an interesting problem... the django code handling swappable models specifically looks at the main django settings for the settings listed on the undocumented swappable meta option and so it was doing getattr(settings, "OTP_WEBAUTHN_CREDENTIAL_MODEL") - the setting name was of course a variable, but this was the effect, and then it would raise an exception. I got curious and totally removed all of the django_otp_webauthn migrations in my test project and tried to create the migrations from scratch, without swapping anything, and ran into the same problem.

So I did some more searching on stack overflow, the django trac system, etc. and finally found very old information stating that the swappable model api is an alpha which was (is?) intended to be made ready for others but currently is pretty much written to specifically work with swapping the user model and so is not documented. Apparently this has not changed in 12 years.

That references this django app, which appears is still being actively developed.

So, hopefully I will get some free time again and take a crack at it using that app.

As a side note, it does also appear that things work if OTP_WEBAUTHN_CREDENTIAL_MODEL and OTP_WEBAUTHN_ATTESTATION_MODEL are just always specified, but that would require users of this package to put those into their settings even to use the default models. There are also a couple of things in the existing migrations which may have issues with that still, although they are in the 0003 migration which has been squashed out.

[jmichalicek]

@Stormheg I've done a bit of playing with that swapper app. The only real gotcha with it that I'm finding so far is that to use it the settings need to become OTP_WEBAUTHNCREDENTIAL_MODEL and OTP_WEBAUTHNATTESTATION_MODEL which isn't terrible, but also isn't ideal or consistent with the other settings.

We could just borrow the code and include it as part of this project, with the adjustments needed to use OTP_WEBAUTHN_CREDENTIAL_MODEL and OTP_WEBAUTHN_ATTESTATION_MODEL.

The default behavior would actually have settings of DJANGO_OTP_WEBAUTHN_WEBAUTHNCREDENTIAL_MODEL, etc as <app_label>_<model_name>_MODEL all uppercase, but it does provide the facilities to override using the app name as the base.

Do you have any preference either way? It's not a huge, complex app, so just having a built in implementation isn't too big of a deal to maintain consistency.

[Stormheg]

Hello again @jmichalicek! Sorry for taking so long to respond to your comments - I've been away on a long holiday to the Alps. Been catching up on work and finally have time to look at this in close detail. I realize I could have at least posted an acknowledgement of the issue, sorry about that...

---

Support for custom models is a feature that I believe is very useful to have, but not something I tested very well as you've discovered...

On swappable models

I've read up on swapping models as I am not super familiar with the concept. I tried experimenting with it some to see how it would work. The result: mixed feelings; I ran into quite a few issues myself. Especially the relation of AbstractWebAuthnCredential to the credential model is problematic, as the migration will depend on the current value of OTP_WEBAUTHN_CREDENTIAL_MODEL, which may end up being a custom model. This causes Django to apply the custom migration before any django_otp_webauthn migrations, which is only the case if you start out with custom models right away. If you don't, and add custom models later, the calculated order in which migrations should apply changes.

Suddenly django_otp_webauthn migrations need to be applied after your own, because they now depend on your custom model. This ends up crashing with InconsistentMigrationHistory.

Maybe there are ways to overcome or prevent the issues I encountered, but overall it all appears very delicate to me. This makes me uncomfortable about using swappable models.

Alternative - requiring custom models, always

An easier and far less sensitive alternative may be to just require users specify a custom attestation + credential model always to bypass the quarks of swapping entirely. Ideally, I would have liked to avoid complicating the initial setup experience, but it might avoid a lot of pain for developers trying to switch later (and support requests for me to answer).

How does this sound to you?

[jmichalicek]

@Stormheg I personally wouldn't worry too much about the requirement for needing to define your own custom models first if you're going to use them. We should of course document that specifically, but that's already the known and accepted case for custom user models and so shouldn't come as too much of a surprise to anyone wanting to do so here.

What you've got in place probably works for 99% of users. I only started to play with it for the idea of having a UUID for the primary key on the credential rather than the default bigint sequential id. We have briefly mentioned multi-site deployments before and that's the main other area I would see someone commonly using a custom model and I'm really not sure just how common multi-site deployments are. In around 14 years of full time Django development for my job, I've worked for one place which did it, interviewed at one other, and then my current employer has had a single client which used it and those are the only times I've seen it in person.

I've worked with one framework built on top of Django which took the "always use your own custom things because we don't know what you need" approach. I'll leave their name out of it for now. It was incredibly flexible while still providing some level of base tooling, but honestly got very tedious and convoluted to work with because once a few models were always all custom all the time, so much other code needed to be as well. Sometimes it felt like there wasn't much point in using the framework.

I'm happy to keep digging into this, give it some serious manual testing with different situations, etc. I pretty much always use a custom user model for Django, so I'm familiar with what I would expect to happen most of the time. The doubling up of potential swappable models does come with a bit of extra potential work for developers making use of that functionality, but only those choosing to do so. It's mentioned in the docs for swapper but I do need to play myself to really understand when it comes into play.

Also, no problem on the slow reply. I knew you were on vacation. This is also something I'm doing sort of for work, but mostly in free time either as I can find some at work or just when it feels right after a day of work between family, hobbies, etc. So I'm rarely in a rush.

[Stormheg]

@jmichalicek I've thought of another solution: how about 'fixing' the relation from AbstractWebAuthnAttestation to django_otp_webauthn.WebAuthnCredential instead of a relying on a dynamic setting? We'll document that if you want to customize the models, you'll have to create both a custom Attestation and Credential model + relation between those. I think that avoids all of the migration pitfalls that come with swappable models and is still flexible enough to make custom models work.

I personally wouldn't worry too much about the requirement for needing to define your own custom models first if you're going to use them. We should of course document that specifically, but that's already the known and accepted case for custom user models and so shouldn't come as too much of a surprise to anyone wanting to do so here.

I do worry about this. A small project that's perfectly okay with the default models sometimes grows beyond the scope of what the default models can provide. When that happens, migration is a pain. I'm not ready yet to dismiss that as an acceptable downside.