-
Notifications
You must be signed in to change notification settings - Fork 3
BA-1808 Add basic profile information to JWT claims #189
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
Changes from all commits
2a7dd04
338feaf
dd0fd03
050e8b0
55fd0c2
23df42d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,13 +26,12 @@ def get_claim_serializer_class(cls): | |
| class BaseJwtLoginSerializer( | ||
| CustomClaimSerializerMixin, LoginPasswordExpirationMixin, TokenObtainPairSerializer | ||
| ): | ||
| @classmethod | ||
| def get_token(cls, user): | ||
| def get_token(self, user): | ||
| token = super().get_token(user) | ||
|
|
||
| # Add custom claims | ||
| if cls._claim_serializer_class: | ||
| data = cls.get_claim_serializer_class()(user).data | ||
| if self._claim_serializer_class: | ||
| data = self.get_claim_serializer_class()(user, context=self.context).data | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This allows relative urls to be converted to absolute urls when serializing the profile image |
||
| for key, value in data.items(): | ||
| token[key] = value | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,11 @@ | ||||||||||||||
| import swapper | ||||||||||||||
|
|
||||||||||||||
| from datetime import timedelta | ||||||||||||||
|
|
||||||||||||||
| from baseapp_auth.utils.referral_utils import get_user_referral_model, use_referrals | ||||||||||||||
| from baseapp_core.graphql import get_obj_relay_id | ||||||||||||||
| from baseapp_core.rest_framework.serializers import ModelSerializer | ||||||||||||||
| from baseapp_core.rest_framework.fields import ThumbnailImageField | ||||||||||||||
| from baseapp_referrals.utils import get_referral_code, get_user_from_referral_code | ||||||||||||||
| from constance import config | ||||||||||||||
| from django.contrib.auth import get_user_model | ||||||||||||||
|
|
@@ -14,30 +18,51 @@ | |||||||||||||
| from .fields import AvatarField | ||||||||||||||
|
|
||||||||||||||
| User = get_user_model() | ||||||||||||||
| Profile = swapper.load_model("baseapp_profiles", "Profile") | ||||||||||||||
|
|
||||||||||||||
| from baseapp_auth.password_validators import apply_password_validators | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| class JWTProfileSerializer(serializers.ModelSerializer): | ||||||||||||||
| """ | ||||||||||||||
| Serializes minimal profile data that will be attached to the JWT token as claim | ||||||||||||||
| """ | ||||||||||||||
| url_path = serializers.SlugField(required=False) | ||||||||||||||
| id = serializers.SerializerMethodField() | ||||||||||||||
| image = ThumbnailImageField(required=False, sizes={"small": (100,100)}) | ||||||||||||||
|
|
||||||||||||||
| class Meta: | ||||||||||||||
| model = Profile | ||||||||||||||
| fields = ("id", "name", "image", "url_path") | ||||||||||||||
|
|
||||||||||||||
| def get_id(self, profile): | ||||||||||||||
| return get_obj_relay_id(profile) | ||||||||||||||
|
|
||||||||||||||
| def to_representation(self, profile): | ||||||||||||||
| data = super().to_representation(profile) | ||||||||||||||
| if (data['image'] != None): | ||||||||||||||
| data['image'] = data['image']['small'] | ||||||||||||||
| return data | ||||||||||||||
|
Comment on lines
+43
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix comparison to None Use - if (data['image'] != None):
+ if data['image'] is not None:📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.8.0)43-43: Comparison to Replace with (E711) |
||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| class UserBaseSerializer(ModelSerializer): | ||||||||||||||
| name = serializers.CharField(required=False, allow_blank=True, source="first_name") | ||||||||||||||
| avatar = AvatarField(required=False, allow_null=True) | ||||||||||||||
| profile = JWTProfileSerializer(read_only=True) | ||||||||||||||
| email_verification_required = serializers.SerializerMethodField() | ||||||||||||||
| referral_code = serializers.SerializerMethodField() | ||||||||||||||
| referred_by_code = serializers.CharField(required=False, allow_blank=True, write_only=True) | ||||||||||||||
|
|
||||||||||||||
| class Meta: | ||||||||||||||
| model = User | ||||||||||||||
| fields = ( | ||||||||||||||
| "id", | ||||||||||||||
| "profile", | ||||||||||||||
| "email", | ||||||||||||||
| "is_email_verified", | ||||||||||||||
| "email_verification_required", | ||||||||||||||
| "new_email", | ||||||||||||||
| "is_new_email_confirmed", | ||||||||||||||
| "referral_code", | ||||||||||||||
| "referred_by_code", | ||||||||||||||
| "avatar", | ||||||||||||||
| "name", | ||||||||||||||
| "phone_number", | ||||||||||||||
| "preferred_language", | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
|
|
||
| class ProfileManager(models.Manager): | ||
| def filter_user_profiles(self, user): | ||
| return self.filter(models.Q(members__user=user) | models.Q(owner=user)) | ||
| return self.filter(models.Q(members__user=user) | models.Q(owner=user)).distinct() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Similar queries found in the codebase - one missing .distinct() The codebase scan reveals an inconsistency in the usage of
🔗 Analysis chainLGTM! The addition of .distinct() prevents duplicate profiles. The change correctly ensures unique profiles when a user is both a member and owner of a profile, which is essential for accurate JWT claims. Let's verify the SQL query generation and check for similar patterns in the codebase: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for similar query patterns and potential inconsistencies
# Look for similar Q-object queries that might need .distinct()
rg -A 2 "models\.Q\(.*\)\s*\|.*models\.Q\(" --type py
# Check for other profile-related queries that might need similar treatment
ast-grep --pattern 'filter($$$).distinct()'
Length of output: 1004 |
||
|
|
||
| def get_if_member(self, user, **kwargs): | ||
| if user.is_superuser: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures that profiles are only displayed once when switching profiles (calling
.filterwith a condition that involves multiple databases might produce duplicate entries)