Corvia / django-tenant-users

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

DRF with Django Tenant Users #679

Open thubamamba opened 1 month ago

thubamamba commented 1 month ago

I have been trying to find an implementation on Django Rest Framework, to no avail. Reading the docs, I don't see anything that I have missed, I am able to create public and additional tenants, however, when testing, users who don't even belong to a tenant are able to create resources for those respective tenants. I really would appreciate the guidance here. :

from django.db import models
from django.utils import timezone
from django_tenants.models import DomainMixin, TenantMixin
from tenant_users.tenants.models import TenantBase

class Company(TenantBase):
    name = models.CharField(max_length=100, unique=True)
    paid_until = models.DateField(null=True, blank=True)
    is_active = models.BooleanField(default=False, blank=True)
    created = models.DateTimeField(default=timezone.now)
    auto_create_schema = True

    class Meta:
        ordering = ('-created',)

    def __str__(self):
        return f"{self.name}"

class Domain(DomainMixin):
    pass

In the custom user, I have:

from django.db import models
from django.core.validators import RegexValidator
from django.utils import timezone
from tenant_users.tenants.models import UserProfile
# Create your models here.
USERNAME_PATTERN = '^[a-z0-9+]{2,25}$'
USERNAME_ERROR_MESSAGE = 'invalid username formate'

class User(UserProfile):
    name = models.CharField(max_length=50)
    email = models.EmailField(max_length=255, unique=True)
    username = models.CharField(max_length=25, unique=True, validators=[
        RegexValidator(regex=USERNAME_PATTERN, message=USERNAME_ERROR_MESSAGE)])
    is_active = models.BooleanField(default=True)
    is_staff = models.BooleanField(default=False)
    is_superuser = models.BooleanField(default=False)
    created_date = models.DateTimeField(default=timezone.now)

    REQUIRED_FIELDS = ['name', 'password', 'email']
    USERNAME_FIELD = 'username'

Worth noting is that I am using SimpleJWT, with Djoser and the following is my config:

SHARED_APPS = [
    "django_tenants",   # Mandatory
    "jazzmin",
    # Everything below here is optional
    'django.contrib.admin',
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'tenant_users.permissions',
    'tenant_users.tenants',
    # Mandatory
    "companies",
    "accounts",
    'django.contrib.sessions',
    'django.contrib.messages',
    'django.contrib.staticfiles',

    # Others apps
    'django_extensions',
    'corsheaders',
    'djoser',
    'drf_yasg',
    'rest_framework',
    'rest_framework.authtoken',
    "rest_framework_simplejwt",
    "storages",
]

TENANT_APPS = [
    "django_tenants",
    # Tenant-specific apps
    'guests',

    # Other apps
    'django.contrib.auth',
    'django.contrib.contenttypes',
    'tenant_users.permissions',

    'rest_framework',
    'rest_framework.authtoken',
    'rest_framework_simplejwt',

    # Installed apps
    "drf_yasg",
    # 'jazzmin',
    'corsheaders',
    # "storages",
]

INSTALLED_APPS = list(SHARED_APPS) + [app for app in TENANT_APPS if app not in SHARED_APPS]

MIDDLEWARE = [
    "django_tenants.middleware.main.TenantMainMiddleware",  # Mandatory, must be 1st
    "corsheaders.middleware.CorsMiddleware",
   .....
]

ROOT_URLCONF = "drf_api.urls"
PUBLIC_SCHEMA_URLCONF = "drf_api.urls_public"

# Multitenancy
TENANT_MODEL = "companies.Company"
TENANT_DOMAIN_MODEL = "companies.Domain"
DEFAULT_FILE_STORAGE = "django_tenants.files.storage.TenantFileSystemStorage"   # each tenant will have a dedicated subdirectory for storing user-uploaded files
MULTITENANT_RELATIVE_MEDIA_ROOT = ""  # (default: create sub-directory for each tenant)
SHOW_PUBLIC_IF_NO_TENANT_FOUND = True  # (default: False) - If True, the public schema will be used if no tenant is found
AUTHENTICATION_BACKENDS = ("tenant_users.permissions.backend.UserBackend",)
TENANT_USERS_ACCESS_ERROR_MESSAGE = "Custom access denied message."
TENANT_USERS_DOMAIN = "localhost:8000"
Dresdn commented 1 month ago

Keep in mind "Authentication != Authorization".

What's your DRF DEFAULT_PERMISSION_CLASSES set to? By default, it's AllowAny.

The django-tenant-users app will by default Authenticate people globally. It's up to you to determine how to Authorize users per tenant. If you want to segment it where a user has full access to the APIs of a given tenant, then take a look at the TenantAccessMiddleware to see one way of doing that, and convert that logic into a DRF PERMISSIONS_CLASSES.

thubamamba commented 1 month ago

Here is my REST_FRAMEWORK config:

REST_FRAMEWORK = {
    "NON_FIELD_ERRORS_KEY": "errors",
    "DEFAULT_AUTHENTICATION_CLASSES": [
        "rest_framework_simplejwt.authentication.JWTAuthentication",
    ],
    "DEFAULT_PERMISSION_CLASSES": [
        "rest_framework.permissions.IsAuthenticated",
    ],
    "DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.PageNumberPagination",
    "PAGE_SIZE": 10,
}

Thanks for the docs, I will peruse and see if I can get it done. Do you maybe have plans to have an example DRF implemention?

Dresdn commented 1 month ago

Do you maybe have plans to have an example DRF implemention?

Not really as it's pretty "vanilla".

If you think it'd be helpful, more than happy to merge in a DRF Permission Class as an example that shows how you can prevent users who are not members (meaning the TenantUser.tenants doesn't contain the given tenant) from accessing the Tenant's APIs. Feel free to open a PR!

thubamamba commented 1 month ago

Do you maybe have plans to have an example DRF implemention?

Not really as it's pretty "vanilla".

If you think it'd be helpful, more than happy to merge in a DRF Permission Class as an example that shows how you can prevent users who are not members (meaning the TenantUser.tenants doesn't contain the given tenant) from accessing the Tenant's APIs. Feel free to open a PR!

Hey @Dresdn, thank you for your willingness to help. I have opened the PR as requested.

Tugark commented 1 month ago

@Dresdn We are facing the same questions right now, so I just wanted to briefly ask whether I fully understood your statements (also in the PR :)).

We are using django-tenants + django-tenants-users + django_restframework + djoser + django_simplejwt. We obtain a JWT and supply it as Header.

We tried adding a Middleware, but in middlewares, we don't have the request.user object. However, in permission classes, we have it. I've seen issues in django_simplejwt, so I assume this is correct.

Now, in order to protect tenant-aware views (i.e. UserA who is assigned to TenantA should be able to access tenanta.myapp.com/api/v1/products, but NOT tenantb.myapp.com/api/v1/products), all we had to do in our proof of concept was to add a custom permission class like so:


class HasTenantAccess(BasePermission):
    def has_permission(self, request: WSGIRequest, view: View) -> bool:
        tenant = get_current_tenant() # typehint is wrong

        return tenant.user_set.filter(id=request.user.id).exists()

This uses your utils class, fetches the tenant and then checks whether the given user is a member of this tenant. Usage is like so:

@permission_classes([IsAuthenticated, HasTenantAccess])
class ProjectViewSet(viewsets.ModelViewSet):
    queryset = Project.objects.all()
    serializer_class = ProjectSerializer

This will disallow anonymous access AND check whether the user is allowed to this tenant. So if UserA accesses this endpoint on the TenantB subdomain, they will receive an error:

{
    "detail": "You do not have permission to perform this action."
}

Now: Is this the proper way how you would envision this and what you meant with your statements?

If so, I'll be happy to create a PR with an example for the documentation on this.

Thank you! :-)

Dresdn commented 1 month ago

That's exactly what I was thinking @Tugark.

The question is: Where to put it? The tenant_users.permissions and tenant_users.tenants modules drive me nuts working with the package (why not put everything just in tenant_users?!?), and I think doing tenant_users.permissions.permissions is silly.

Curious what you come up with!