Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Reading role permissions generates 500 error backend #3079

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions api/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.conf import settings
from django.contrib.auth.base_user import BaseUserManager
from django.contrib.auth.models import AbstractUser
from django.core.exceptions import ImproperlyConfigured
from django.core.mail import send_mail
from django.db import models
from django.db.models import Count, QuerySet
Expand Down Expand Up @@ -42,6 +43,9 @@
InviteLink,
)

if settings.IS_RBAC_INSTALLED:
from rbac.models import UserRole

logger = logging.getLogger(__name__)
mailer_lite = MailerLite()

Expand Down Expand Up @@ -243,6 +247,14 @@ def get_user_organisation(
% (self.id, getattr(organisation, "id", organisation))
)

def get_user_roles(self):
if not settings.IS_RBAC_INSTALLED:
raise ImproperlyConfigured(
"RBAC is not installed. Unable to retrieve user roles."
)

return UserRole.objects.filter(user=self)

def get_permitted_projects(
self, permission_key: str, tag_ids: typing.List[int] = None
) -> QuerySet[Project]:
Expand Down
12 changes: 11 additions & 1 deletion api/users/serializers.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from django.conf import settings
from djoser.serializers import UserSerializer as DjoserUserSerializer
from rest_framework import serializers
from rest_framework.exceptions import ValidationError

from organisations.models import Organisation
from organisations.serializers import UserOrganisationSerializer

if settings.IS_RBAC_INSTALLED:
from rbac.serializers import UserRoleSerializer

from .models import FFAdminUser, UserPermissionGroup


Expand Down Expand Up @@ -56,9 +60,15 @@ class Meta:
class UserListSerializer(serializers.ModelSerializer):
role = serializers.SerializerMethodField(read_only=True)
join_date = serializers.SerializerMethodField(read_only=True)
if settings.IS_RBAC_INSTALLED:
roles = UserRoleSerializer(many=True, read_only=True, source="get_user_roles")
Comment on lines +63 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like the if-statement that leads to roles not being set on the serializer. Is this how we do it throughout the codebase? I feel like defaulting roles to None would look better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is possible since UserRoleSerializer lives in RBAC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok makes sense.


default_fields = ("id", "email", "first_name", "last_name", "last_login")
organisation_users_fields = ("role", "date_joined")
organisation_users_fields = (
"role",
"date_joined",
*([] if not settings.IS_RBAC_INSTALLED else ["roles"]),
)

class Meta:
model = FFAdminUser
Expand Down