fix: preserve SCIM userName for Okta integration #42253
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
ee/api/scim/views.py
Outdated
| SCIM_USER_ATTR_MAP = { | ||
| ("userName", None, None): "email", |
There was a problem hiding this comment.
logic: userName now maps to SCIMProvisionedUser.username instead of User.email, but this filter mapping still points to email. This breaks case-sensitive filtering. When Okta filters by userName eq "[email protected]", it searches User.email (lowercased) instead of SCIMProvisionedUser.username (preserves case).
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/api/scim/views.py
Line: 28:29
Comment:
**logic:** `userName` now maps to `SCIMProvisionedUser.username` instead of `User.email`, but this filter mapping still points to `email`. This breaks case-sensitive filtering. When Okta filters by `userName eq "[email protected]"`, it searches `User.email` (lowercased) instead of `SCIMProvisionedUser.username` (preserves case).
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Legit comment, will fix it
ee/api/scim/views.py
Outdated
| # Check if user is already SCIM-provisioned for this org domain | ||
| email = PostHogSCIMUser._extract_email_from_value(request.data.get("emails", [])) | ||
| if email: | ||
| user = User.objects.filter(email__iexact=email).first() | ||
| if user and SCIMProvisionedUser.objects.filter(user=user, organization_domain=organization_domain).exists(): | ||
| return Response( | ||
| {"schemas": [constants.SchemaURI.ERROR], "status": 409, "detail": "User already exists"}, | ||
| status=status.HTTP_409_CONFLICT, | ||
| ) |
There was a problem hiding this comment.
logic: Race condition: the 409 check happens outside the transaction in from_dict(). Two concurrent POST requests with the same email could both pass this check, then one would succeed and the other would hit a database constraint error instead of returning 409.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/api/scim/views.py
Line: 148:156
Comment:
**logic:** Race condition: the 409 check happens outside the transaction in `from_dict()`. Two concurrent POST requests with the same email could both pass this check, then one would succeed and the other would hit a database constraint error instead of returning 409.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Moved the check if user is already provisioned into transaction
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| migrations.swappable_dependency(settings.AUTH_USER_MODEL), | ||
| ("posthog", "0923_add_quick_filters"), | ||
| ("ee", "0031_agentartifact"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name="SCIMProvisionedUser", | ||
| fields=[ | ||
| ("id", models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), | ||
| ("updated_at", models.DateTimeField(auto_now=True, null=True)), | ||
| ( | ||
| "identity_provider", | ||
| models.CharField( | ||
| choices=[ | ||
| ("okta", "Okta"), | ||
| ("entra_id", "Microsoft Entra ID"), | ||
| ("google", "Google Workspace"), | ||
| ("onelogin", "OneLogin"), | ||
| ("other", "Other"), | ||
| ], | ||
| max_length=50, | ||
| ), | ||
| ), | ||
| ("username", models.CharField(max_length=255)), | ||
| ("active", models.BooleanField(default=True)), | ||
| ("created_at", models.DateTimeField(auto_now_add=True)), | ||
| ( | ||
| "organization_domain", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="scim_provisioned_users", | ||
| to="posthog.organizationdomain", | ||
| ), | ||
| ), | ||
| ( | ||
| "user", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="scim_provisions", | ||
| to=settings.AUTH_USER_MODEL, | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "indexes": [ | ||
| models.Index(fields=["organization_domain", "username"], name="ee_scimprov_organiz_6d83ff_idx") | ||
| ], | ||
| }, | ||
| ), | ||
| migrations.AddConstraint( | ||
| model_name="scimprovisioneduser", | ||
| constraint=models.UniqueConstraint( | ||
| fields=("user", "organization_domain"), name="unique_user_organization_domain" | ||
| ), | ||
| ), | ||
| ] |
There was a problem hiding this comment.
logic: Missing data migration for existing SCIM-provisioned users. Users created before this PR won't have SCIMProvisionedUser records, so their userName will fallback to lowercased email and they can't be filtered by case-sensitive userName.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/migrations/0032_scimprovisioneduser_and_more.py
Line: 8:66
Comment:
**logic:** Missing data migration for existing SCIM-provisioned users. Users created before this PR won't have `SCIMProvisionedUser` records, so their `userName` will fallback to lowercased email and they can't be filtered by case-sensitive userName.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
We don't have any customers on SCIM yet, so this is not critical
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 0 Needs Review | 0 Blocked ✅ SafeNo contention risk, backwards compatible Last updated: 2025-11-28 19:31 UTC (ebc83a1) |
Problem
Okta integration tests were failing because SCIM expects userName to preserve case, but we were returning the user's email which gets lowercased.
Changes
userName,is_active, andidentity_providerscim_provisioned_userinstead of theuser.email, preserving the original caseHow did you test this code?
Tests + Runscope: https://www.runscope.com/radar/lx4oulj4u3qz/e9d81345-46fd-414a-b833-34241e91d956/history/ed52c993-0a64-4299-b5d4-d2cfbdd736d7