Do not use a custom StormpathPermissionsMixin class as it confuses the ORM

[davidmarquis]

Not sure about the original reason for using a custom StormpathPermissionsMixin mixin class in the StormpathBaseUser hierarchy, but it confuses the ORM while creating the tables for storing Django users. Instead of creating a single table for storing StormpathUser objects, it creates 4 tables:

• django_stormpath_stormpathuser
• django_stormpath_stormpathpermissionsmixin
• django_stormpath_stormpathpermissionsmixin_groups
• django_stormpath_stormpathpermissionsmixin_user_permissions

The django_stormpath_stormpathpermissionsmixin table references the first using a foreign key and essentially adds a single field is_superuser and all relations get mapped from the django_stormpath_stormpathpermissionsmixin table.

This might seem harmless but it's actually quite bad: in a use case where you need to list users via a "related" property (ex: group.user_set), things go completely south as the ORM is confused and thinks it needs to load objects of type StormpathPermissionsMixin, whereas it should load StormpathUser objects.

My particular use case is with Django CMS, which tried to access the users of a given group using group.user_set and assumes derivatives of the standard AbstractBaseUser should be returned, leading to an error with an unresolvable property - in this specific case, it's trying to get the email property on StormpathPermissionsMixin, which obviously does not exist.

By changing the parents of StormpathBaseUser to (AbstractBaseUser, PermissionsMixin), the ORM is happy and combines all fields in a single table (django_stormpath_stormpathuser). Moreover, the 'defective' Django CMS code now works perfectly.

Please note that this change will be backward incompatible to existing users of the django_stormpath app, but it seems warranted.

Thanks!

[omgitstom]

Hi @davidmarquis - thanks for these, we love community contributions! Our resident python expert @rdegges is at pycon right now, but I'm sure he will review these when he is back next week. We follow semantic versioning @ Stormpath, so we may have to move this change in particular major version since it may break backwards compatibility.

[davidmarquis]

Thanks @omgitstom! Looking forward to hear more from @rdegges

[rdegges]

Hey @davidmarquis. This makes perfect sense. I 100% agree.

What I'll do is this: I'm going to cut a new release of the current 0.x.x releases with the other stuff merged in -- then release a new 1.x.x which includes this (backwards-incompatible) change.

I think that's reasonable, especially since this is a new library.

[davidmarquis]

Excellent. Thanks Randall!

[davidmarquis]

@rdegges just wanted to get a quick update on when you think you'll be able to release the updated library version with this change included?

[rdegges]

Done =)

[davidmarquis]

Great! Thanks will try it out