Conversation
No invitation email for now
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds admin and API paths to create “passwordless” stub users (email-only, unusable password, null Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant AdminUI as Admin Interface
participant Backend as Backend
participant DB as Database
Admin->>AdminUI: Open user changelist
AdminUI->>Backend: GET changelist
Backend->>DB: Query users
DB-->>Backend: Users list
Backend-->>AdminUI: Render changelist (add "Add passwordless")
Admin->>AdminUI: Click "Add passwordless"
AdminUI->>Backend: GET add-passwordless form
Backend-->>AdminUI: Render form
Admin->>AdminUI: Submit email (POST)
AdminUI->>Backend: POST email
Backend->>DB: Check email uniqueness
DB-->>Backend: Not found
Backend->>DB: Create User (unusable password, sub=NULL)
DB-->>Backend: User created
Backend-->>AdminUI: Redirect to changelist with message
sequenceDiagram
participant Client
participant API as API Endpoint
participant Serializer
participant DB as Database
participant OIDC as OIDC Backend
Client->>API: POST access request (user=email@example.com)
API->>Serializer: Validate user field (UserAccessWriteField)
Serializer->>DB: Query User by email
DB-->>Serializer: Not found
Serializer->>DB: Create stub User (sub=NULL, unusable pwd)
DB-->>Serializer: Stub created
Serializer-->>API: Validated data
API->>DB: Create access record linked to stub
DB-->>API: Access created
API-->>Client: 201 Created
Client->>OIDC: OIDC login with same email (payload has sub)
OIDC->>DB: Query by sub
DB-->>OIDC: Not found
OIDC->>DB: Query by email AND sub IS NULL
DB-->>OIDC: Stub user found
OIDC->>DB: Update user.sub with payload.sub
DB-->>OIDC: User updated (stub claimed)
OIDC-->>Client: Authentication successful
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/core/admin.py`:
- Around line 242-271: The add_passwordless_view currently allows access without
enforcing model-level add rights; at the top of add_passwordless_view check
self.has_add_permission(request) and if it returns False deny access (e.g.,
raise django.core.exceptions.PermissionDenied or return HttpResponseForbidden)
before processing POST or rendering the form so only users with add permission
on the User model can create passwordless users; this uses the existing
add_passwordless_view method and self.has_add_permission to gate the endpoint.
In `@src/backend/core/api/serializers.py`:
- Around line 1098-1104: The current lookup uses
models.User.objects.filter(email=data).first() which silently picks one record
when multiple users share the same email; change the logic to explicitly detect
ambiguous matches: query = models.User.objects.filter(email=data), if
query.count() > 1 raise a validation/error (e.g., serializers.ValidationError or
appropriate API error) requiring a UUID instead of an email, if query.count() ==
1 return that user, and only create a new user (user = models.User(email=data);
set_unusable_password(); user.save()) when query.count() == 0; ensure the error
path prevents granting access when email is ambiguous.
In `@src/backend/core/models.py`:
- Around line 123-126: The call to self.get(email=email, sub__isnull=True) can
raise self.model.MultipleObjectsReturned for duplicate stub-email rows; modify
the try/except to also catch self.model.MultipleObjectsReturned and handle it by
selecting a single canonical row (e.g., use self.filter(email=email,
sub__isnull=True).order_by('pk').first()) or otherwise resolve/log and return
that single instance (or None) instead of letting the exception bubble; update
the exception handler around the self.get(...) call to perform this safe
fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0206867a-1999-442c-8dfa-b7d6cfafb2d7
📒 Files selected for processing (8)
src/backend/core/admin.pysrc/backend/core/api/serializers.pysrc/backend/core/models.pysrc/backend/core/templates/admin/core/user/add_passwordless.htmlsrc/backend/core/templates/admin/core/user/change_list.htmlsrc/backend/core/tests/api/test_mailbox_access.pysrc/backend/core/tests/api/test_maildomain_access.pysrc/backend/core/tests/authentication/test_backends.py
No invitation email for now
Summary by CodeRabbit
New Features
Tests