Corvia / django-tenant-users

Adds global user authentication and tenant-specific permissions to django-tenants.
https://django-tenant-users.rtfd.io
MIT License
328 stars 63 forks source link

cannot install on an existing django_tenants project #602

Open HansvanHespen opened 2 months ago

HansvanHespen commented 2 months ago

I'm giving up. I work now for six months with django_tenants. The package is far from perfect. I use subfolders for my tenants, and that doesn't work. I had to write my own middleware to take out the errors and make it work, and also had to rewrite urlresolvers.py and create a context_processors.py. I mentioned all that to Tom Turner, administrator of django_tenants, and asked for a pull request, but a year later, nothing has been done.

Unfortunately, I also dicovered that django_tenants has problems with the users. When a user logs in in a tenant, by the next html page, it has forgotten about the user. Only in the admin section of the tenant, it holds. God knows why.

Last but not least, the superuser of public has no access to its tenants, which I solved by automatically adding the public superuser to every tenant at its creation. Not ideal, because now the superuser has to log on every time he changes from tenant, but at least, we have access.

And now, when a new tenant is created, I have an error since it has forgotten already about it's user. So I need a solution there.

I thought that django_tenant_users was going to be the solution. It offers a overall superuser with single sign-on, and by using it's own user model instead of django's, I hoped to get rid of the user amnesia problems of django_tenants.

However, although installing django_tenant_users went well thanks to the extensive documentation, making it work turned out to be a nightmare. I ended up wiping my existing database and start all over again, but still have dependency problems. Even though the documentation says that I first need to have a fully installed and configured django_tenants project, which I have.

I've got two main problems. When inheriting my tenant model from TenantBase, and then do makemigrations, it asks for a default value for the owner, since TenantBase requires an owner that is non-nullable. This owner should be a pointer to an object, which is difficult to type in like that in the vscode terminal when requested. I tried some real code, as is suggested, but that doesn't work. I ended up by just typing '1', well knowing of course that it won't go through migrate.

image

I first need the user model defined in the database, then I need to create a user in the database, and only then I can connect a user to the owner field of my tenantmodel. Then, I have to adapt the tenant model by adding a default value to the owner. However, since the owner field is added in the TenantBase class and therefore not my own code, I have to adapt the TenantBase class with my personal code. I can do that, but i shouldn't, because when a new version of django_tenant_users comes out, it will overwrite my code. Anyway, all of that is not mentioned in the documentation that I should do these kind of things. Believing the docs, it should be a walk in the park. Change the baseclass in the models.py, some stuff in the settings.py, create a new users app and a model in it inheriting from UserProfile, and that's it. Not my experience.

The second problem is the dependencies. I always have this error: django.db.migrations.exceptions.InconsistentMigrationHistory: Migration admin.0001_initial is applied before its dependency csb_users.0001_initial on database 'default'.

So, my idea was to first do a makemigrations without my tenant model inheriting from TenantBase, so I would have the user model already. Wrong idea. It complains about the admin migration being done before the migration of the user model. Which to some extent makes sense, but the doc says explicitly that django_tenants should be configured first! So, I did the migrate_schema --shared, created a public tenant and a superuser for it as the doc explicitly says.

I have gone into the django_migrations table and took out the migrations, wiped out the migrations in my django project, tried again.

That's the reason why I ruined my existing database in the first place. When I first tried to install django_tenant_users over my existing django_tenants project and database, it pointed to this admin migration, that was number three of my django_migrations table, and so, I wiped out all of the migrations. Then I had to wipe out all corresponding migrations in my django project. Then the new migrations still had other problems. So, i decided to wipe all of the tenants, clean the public schema and finally delete the public schema as well. Now, i'm building up everything again, but i'm having this problem django.db.migrations.exceptions.InconsistentMigrationHistory: Migration admin.0001_initial is applied before its dependency csb_users.0001_initial on database 'default' again on a blank database with an intial configuration of django_tenants as mentioned in the doc. I'm already days working on this, and haven't gotten anywhere.

And when I will have to put the new version of my software on the cloud, I will have to go through this misery again on my existing database in the cloud.

I'm wasting tremendous lots of time here, the doc is at least not complete, and I start having the feeling that this package is as buggy as is django_tenants.

It is also difficult that python manage.py something always boots up my full code. It cannot work because there is no database anymore. So, I had to put that code in remark, then it complained about all of the imports that didn't work. I had to put that in remark as well. So, I ended up ruining my code, my database, no result, no solution for my error with django_tenants, wasted a lot of time.

I give up.

Thank you for listening to my whereabouts.

Expected Behavior

walk in the park

Actual Behavior

errors all over the place: dependency problems in the migrations and on the owner added field.

Possible Fix

It is clear that this software has been created and tested on something different than I have. But what I have is as stated in the docs. I might try to wipe out the database again, not install a public tenant, adapt the code in the TenantBase. Make some manual alterations in the database. Not guaranteed that this will work. How about the public tenant and its user? Does this user needs to come from the user model then? The admin migration will always be first. I can take out the dependencies of the migrations, maybe that will work. But in the end, I have to do it all over on my cloud database. And there, it has to work. I cannot delete it and loose all of the data, I cannot leave the database out of service for a week.

Steps to Reproduce

I have given the screenshots of my vscode terminal. You can have my database on request, but as I pointed out, I wiped out my whole database and started from scratch.

Your Environment

Windows 11 pro, python 3.10.14, django_tenants, postgres db with psycopg2, at least 100 other packages installed like torch, langchain, django, ...cloud is on fly.io. My environment for this problem is pretty straightforward.

Dresdn commented 2 months ago

There's a lot to unpack here, but overall, having a long rant with multiple challenges and then criticizing the maintainers and quality of the applications is probably not the best move.

That said, I'm a sucker for helping, so here I am. The django_tenant_users app requires you to modify your UserModel. Since I'm not sure how experienced you are in Django, I'll let you read this directly from the Django Project: Changing to a custom user model mid-project

To me, that's what this sounds like you're doing, which means you're pretty much going to be on your own and there's so much that's specific to your project and design.

To address some comments in this issue:

When a user logs in in a tenant, by the next html page, it has forgotten about the user.

Sounds like your session setup isn't right and/or something is going on with cookies in the browser. Lots to check there.

I thought that django_tenant_users was going to be the solution. It offers a overall superuser with single sign-on, and by using it's own user model instead of django's, I hoped to get rid of the user amnesia problems of django_tenants.

Your "users won't stay logged in" has nothing to do with the UserModel. This is purely something with sessions. The django_tenant_users package is purely to allow multiple tenants to use the same authentication profile across the entire project while having tenant-specific permissions.

I have given the screenshots of my vscode terminal.

You gave ONE screenshot of a migration that's asking for a default value on a field since you didn't provide one. Since the field is named created and is a DateTime field, the integer 1 probably isn't the best value.

Thank you for listening to my whereabouts.

As a maintainer, I don't want to listen to your whereabouts if it's unproductive. Everyone volunteers for this, and having a long rant that provides nothing of value isn't productive. If you genuinely want some assistance, people are here to help, but having a clear ask and a good attitude goes a long way.

HansvanHespen commented 2 months ago

Dear Dresdn,

thank you for your email.

I appreciate your time and effort.

You find my email not very productive, allow me to disagree. I did my best to be specific and objective in explaining the two problems that I had with installing the package. It might have a bit under snowed by my expression of my displeasement. Fair enough.

You mention that by responding 1 in a timestamp created field, isn't going to help. You're right about that. However, I choose option 1, which is the default 'now'-timestamp.

So, put it differently, in order to stay productive:

  1. Suggestion. For me , it wasn't clear if this package is installable on an existing database or not. The more, since django_tenants need to be pre-installed and configured, I thought there was a good chance that it is indeed possible to install django_tenant_users over an existing django_tenants and database. I think it would be good to take into the documentation a small section about this. Maybe point to your link with regard of changing the users mid-flight in django.

  2. Suggestion. I was surprised that the software asked about defaults for foreign keys on an empty database. Now, given this is the way how it works in Python, it means the user will have to override in it's tenant class the owner field, and add code to default it. As well in any subsidiary app, as in the domain class of the tenant app. For instance:

    
    def get_default_tenant():
    return csbtenants.objects.order_by('id').first().id

class Domain(DomainMixin): tenant = models.ForeignKey(TENANT_MODEL, db_index=True, related_name='domains', on_delete=models.CASCADE, default=get_default_tenant)


  You could add this to your documentation, since, now it isn't complete, or you could add such a thing to the TenantBase class.

3. **Question** Even from an empty database, just by installing from scratch django tenants, i wasn't able to properly install the package django_tenant_users, with this error during migrate that it cannot install:

Migration admin.0001_initial is applied before its dependency csb_users.0001_initial

  If it happens during a scratch install on an empty database installed as stated in the docs, how come you don't have that?

4. **Question.**  That the problem for the user amnsesia with django_tenants can lie in the django sessions, I do agree. When I use the payload of django sessions (django store), then it doesn't hold that info either. This has nothing to do of course with django_tenant_users, but do you have an idea what to look for?

5. **Suggestion.** In the section of your documentation: "using django_tenant_users" / "provisioning a new tenant", the doc says: "To set up a new tenant in your application, utilize tasks.create_public_tenant()" but in the example, it says "provision_tenant. " Maybe some historic oversight?

I'm not paid for my work either, this is just for the sake of the better good of software.

Thank you very much for your effort. Much appreciated. Always eager to learn. 

High regards,

Hans 

By the way, this is my tenant class. Nothing special there, I guess:

class csbtenants(TenantBase): companyname = models.CharField(max_length=100, default = '') IsCompany = models.BooleanField(default=False) name = models.CharField(max_length=100) schema_name = models.CharField(max_length=100)

Dresdn commented 2 months ago

You find my email not very productive, allow me to disagree.

We'll have to agree to disagree on this.

You mention that by responding 1 in a timestamp created field, isn't going to help. You're right about that. However, I choose option 1, which is the default 'now'-timestamp.

I missed that reading the screenshot.

1. **Suggestion.** For me , it wasn't clear if this package is installable on an existing database or not. The more, since django_tenants need to be pre-installed and configured, I thought there was a good chance that it is indeed possible to install django_tenant_users over an existing django_tenants and database. I think it would be good to take into the documentation a small section about this. Maybe point to your link with regard of changing the users mid-flight in django.

This is more of a core Django principle versus something specific to this package. If you already are using a custom user model before this project, I really am not sure the impact of moving from one custom user model to another. If the docs can be improved to help clarify, feel free to submit a PR.

2. **Suggestion.** I was surprised that the software asked about defaults for foreign keys on an empty database. 
You could add this to your documentation, since, now it isn't complete, or you could add such a thing to the TenantBase class.

Again, PRs are welcome.

3. **Question** Even from an empty database, just by installing from scratch django tenants, i wasn't able to properly install the package django_tenant_users, with this error during migrate that it cannot install:
 Migration admin.0001_initial is applied before its dependency csb_users.0001_initial 

If it happens during a scratch install on an empty database installed as stated in the docs, how come you don't have that?

This probably means you have a circular dependency somewhere in your migrations. Not really something this project can control. I would suggest trying a fresh project with all the apps and basic models to ensure your migrations will work.

4. **Question.**  That the problem for the user amnsesia with django_tenants can lie in the django sessions, I do agree. When I use the payload of django sessions (django store), then it doesn't hold that info either. This has nothing to do of course with django_tenant_users, but do you have an idea what to look for?

I would start with the SESSION_COOKIE_DOMAIN setting.

5. **Suggestion.** In the section of your documentation: "using django_tenant_users" / "provisioning a new tenant", the doc says: "To set up a new tenant in your application, utilize tasks.create_public_tenant()" but in the example, it says "provision_tenant. " Maybe some historic oversight?

Probably a typo. I thought there was a PR to fix it already.

Wizely99 commented 1 month ago

@HansvanHespen Have you solved your problem? If not I am willing to help by setting up a new django_tenant project using subfolders and try to reproduce the issues you are having.

HansvanHespen commented 3 weeks ago

@Wizely99 ,

thank you for your offer.

Finally, I did solve the problem I had with django_tenants. @Dresdn was right when he said that there was a problem with the sessions. The sessions however, were properly installed. It's just that django_tenants needs a huge amount of code extra to make it work properly. I wasted about a month of research and coding over it.

I explained it all here: https://stackoverflow.com/questions/78486438/django-tenants-i-spent-a-lot-of-time-on-it, where I answer on someone having the same problems as I had.

The problem or better said: inconvenience, where a general superuser has no access to the tenants, is still not provided with django_tenants. For that, you need this package django_tenant_users. But without the mods I did on django_tenants, you cannot work with this package either. Hence the reason why I mention it here.

Thank you so much for your kind attention.

High regards,

Hans

Wizely99 commented 1 week ago

Hi @HansvanHespen, Thank you for the update and for sharing your experience. I'm glad to hear that you were able to solve the issue, even though it required a significant amount of work. It’s really helpful that you've detailed your solution on Stack Overflow—I'm sure it will assist others who encounter the same challenges with django_tenants.

HansvanHespen commented 1 week ago

You're welcome. That was indeed the reason why i did it, as my contribution to the community.