fix(auth): allow invite-based signup when PREVENT_SIGNUP is enabled - #8408
fix(auth): allow invite-based signup when PREVENT_SIGNUP is enabled#8408bardock-2393 wants to merge 1 commit into
Conversation
IsSignupAllowed blocked every registration once PREVENT_SIGNUP was on, including ones carrying a valid invite link or invited email, so the serializer's own invite validation never even ran. Self-hosted admins who enable PREVENT_SIGNUP to stop self-serve signup could no longer onboard teammates via invite. The invite check is now shared between the permission gate and the serializer instead of duplicated.
|
@bardock-2393 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe change adds a shared registration invite validation service. Signup permission uses it when signup is prevented. Registration serialisation uses the same service before accepting an invite. The service validates invite links by hash and invite emails by case-insensitive email. Unit tests cover unrestricted signup, blocked requests, valid and invalid invite links, and valid invite-email access. Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟠 High · up to This change can let users register with expired invitation links even when administrators have disabled signup, and malformed invited-email input can trigger a server error. Merge should wait until expired links are rejected and email input is safely validated. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 29729aa9-7e87-458d-a9bd-186c9a285cec
📒 Files selected for processing (4)
api/custom_auth/permissions.pyapi/custom_auth/serializers.pyapi/organisations/invites/services.pyapi/tests/unit/custom_auth/test_unit_custom_auth_permissions.py
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| # signup, not registration via an invite link or invited email. | ||
| return is_valid_registration_invite( | ||
| sign_up_type=request.data.get("sign_up_type"), | ||
| email=request.data.get("email") or "", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Normalise raw email input before invite validation.
When PREVENT_SIGNUP is enabled, a request with sign_up_type set to invited email and "email": ["user@example.com"] passes this truthy list to is_valid_registration_invite. The service then calls email.lower() and raises AttributeError before serializer validation. Reject non-string values so this request returns False instead of a server error.
Proposed fix
+ email = request.data.get("email")
return is_valid_registration_invite(
sign_up_type=request.data.get("sign_up_type"),
- email=request.data.get("email") or "",
+ email=email if isinstance(email, str) else "",
invite_hash=request.data.get("invite_hash"),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| email=request.data.get("email") or "", | |
| email = request.data.get("email") | |
| return is_valid_registration_invite( | |
| sign_up_type=request.data.get("sign_up_type"), | |
| email=email if isinstance(email, str) else "", | |
| invite_hash=request.data.get("invite_hash"), | |
| ) |
| ) -> bool: | ||
| match sign_up_type: | ||
| case SignUpType.INVITE_LINK.value: | ||
| return InviteLink.objects.filter(hash=invite_hash).exists() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C5 'class AbstractBaseInviteModel|class InviteLink|objects\s*=|class .*Manager|expires_at|is_expired' api
rg -n -C6 'is_valid_registration_invite|PREVENT_SIGNUP|_validate_registration_invite' api/custom_auth api/organisationsRepository: Flagsmith/flagsmith
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- invite models ---'
sed -n '1,65p' api/organisations/invites/models.py
printf '%s\n' '--- shared registration checks ---'
sed -n '1,80p' api/custom_auth/permissions.py
sed -n '1,60p' api/custom_auth/serializers.py
printf '%s\n' '--- scoped review conventions and learnings ---'
find /tmp/coderabbit-repo-knowledge/flagsmith-flagsmith-a74d157a -type f \
\( -path '*/api/*' -o -path '*/organisations/*' -o -path '*/custom_auth/*' \) \
-name '*.md' -printRepository: Flagsmith/flagsmith
Length of output: 6009
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,55p' api/core/models.py
sed -n '1,30p' api/organisations/invites/services.pyRepository: Flagsmith/flagsmith
Length of output: 2214
Authorization Bypass (CWE-862): Missing Authorization
Reachability: External · Exploitability: Moderate
Exclude expired invite links from registration validation.
InviteLink.objects uses an unfiltered manager, so this hash-only check accepts expired links. Filter for active links and add an expired-link regression test.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8408 +/- ##
==========================================
- Coverage 98.81% 98.67% -0.14%
==========================================
Files 1621 1623 +2
Lines 66102 66142 +40
==========================================
- Hits 65318 65268 -50
- Misses 784 874 +90 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
IsSignupAllowedblocked every registration oncePREVENT_SIGNUPwas enabled, including ones carrying a valid invite link or an invited email address. That meant the invite-validation logic already present in the registration serializer never even ran — self-hosted admins who turn onPREVENT_SIGNUPto stop self-serve signup could no longer onboard teammates via invite link either.IsSignupAllowednow allows a registration request through when it carries a valid invite (link hash or invited email), matching the existing invite-validation behaviour used elsewhere during registration.organisations/invites/services.py) so the permission gate and the serializer use the same logic instead of two separate implementations.Closes #5378
How did you test this code?
Added unit tests for
IsSignupAllowedcovering:PREVENT_SIGNUPoff,PREVENT_SIGNUPon with no invite, with a valid invite link, with an invalid/unknown invite hash, and with a valid invited email. Ran the fullcustom_authserializer + permission test suite against a real local Postgres — all passing. Also ranmypyand the repo'spre-commithooks (lint, format, doc generation, typecheck) clean.Review effort: 2/5