-
Notifications
You must be signed in to change notification settings - Fork 540
feat: add a button to resend an invitation email to a user including a representative for tprm #2894
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a complete invitation feature to the IAM system. Backend introduces two new API views for sending invitation and password-reset emails to local users when email server is configured. Frontend adds a UI button in the representatives detail view and a server route to forward invitation requests to the backend. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as DetailView UI
participant Modal as ConfirmModal
participant Frontend as Server Route
participant Backend as IAM Backend
participant Email as Email Server
User->>UI: Click "Send invitation"
UI->>Modal: Show confirmation modal
User->>Modal: Confirm with email
Modal->>Frontend: POST /representatives/send-invitation/
Frontend->>Frontend: Extract email from formData
Frontend->>Backend: POST /iam/send-invitation/ (email)
Backend->>Backend: Verify user exists
alt Email server configured
Backend->>Email: Send invitation email
Email-->>Backend: Delivery status
Backend-->>Frontend: 202 Accepted
else Email server not configured or user not found
Backend-->>Frontend: 500 Error
end
Frontend-->>Modal: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/iam/urls.py(2 hunks)backend/iam/views.py(1 hunks)frontend/src/lib/components/DetailView/DetailView.svelte(2 hunks)frontend/src/routes/(app)/(third-party)/representatives/send-invitation/+server.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/routes/(app)/(third-party)/representatives/send-invitation/+server.ts (1)
frontend/src/lib/utils/constants.ts (1)
BASE_API_URL(4-8)
backend/iam/urls.py (1)
backend/iam/views.py (1)
SendInvtationView(270-298)
backend/iam/views.py (2)
backend/iam/models.py (2)
is_local(754-771)mailing(631-698)backend/core/views.py (1)
mailing(6493-6514)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
🔇 Additional comments (1)
backend/iam/urls.py (1)
12-15: Routing for invitation endpoint looks consistentThe new
send-invitation/URL is correctly wired to the view and named consistently with the feature; no issues from a routing standpoint.Also applies to: 29-30
| class SendInvtationView(views.APIView): | ||
| permission_classes = [permissions.AllowAny] | ||
|
|
||
| @method_decorator(ensure_csrf_cookie) | ||
| def post(self, request): | ||
| email = request.data["email"] # type: ignore | ||
| associated_user = User.objects.filter(email=email).first() | ||
| if associated_user is None: | ||
| return Response( | ||
| status=HTTP_500_INTERNAL_SERVER_ERROR, | ||
| data={"error": "No user associated with this email"}, | ||
| ) | ||
| if EMAIL_HOST or EMAIL_HOST_RESCUE: | ||
| if associated_user is not None and associated_user.is_local: | ||
| try: | ||
| associated_user.mailing( | ||
| email_template_name="registration/first_connection_email.html", | ||
| subject=_("CISO Assistant: Invitation"), | ||
| ) | ||
| print("Sending invitation mail to", email) | ||
| except Exception as e: | ||
| print(e) | ||
| return Response(status=HTTP_202_ACCEPTED) | ||
| return Response( | ||
| data={ | ||
| "error": "Email server not configured, please contact your administrator" | ||
| }, | ||
| status=HTTP_500_INTERNAL_SERVER_ERROR, | ||
| ) |
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.
Harden SendInvtationView: input validation, status codes, and info leakage
A few points to address here:
- Input validation:
request.data["email"]will raise if the field is missing or malformed. Preferrequest.data.get("email")with a400response when it’s absent/invalid. - Status code for unknown user: returning
HTTP_500_INTERNAL_SERVER_ERRORfor “No user associated with this email” is misleading; it’s a client issue. Also, this message + distinct status leaks whether an email exists, unlikePasswordResetView, which avoids that. Consider always returning202for non‑local/non‑existing users to avoid email enumeration. - Logging:
print(...)should be replaced bylogger.info/logger.errorfor consistency with the rest of the file. - Naming:
SendInvtationViewhas a typo; renaming it toSendInvitationViewwill avoid confusion (remember to update imports/URL mapping).
An example of a more robust implementation:
-class SendInvtationView(views.APIView):
+class SendInvitationView(views.APIView):
permission_classes = [permissions.AllowAny]
@method_decorator(ensure_csrf_cookie)
def post(self, request):
- email = request.data["email"] # type: ignore
- associated_user = User.objects.filter(email=email).first()
- if associated_user is None:
- return Response(
- status=HTTP_500_INTERNAL_SERVER_ERROR,
- data={"error": "No user associated with this email"},
- )
- if EMAIL_HOST or EMAIL_HOST_RESCUE:
- if associated_user is not None and associated_user.is_local:
- try:
- associated_user.mailing(
- email_template_name="registration/first_connection_email.html",
- subject=_("CISO Assistant: Invitation"),
- )
- print("Sending invitation mail to", email)
- except Exception as e:
- print(e)
- return Response(status=HTTP_202_ACCEPTED)
- return Response(
- data={
- "error": "Email server not configured, please contact your administrator"
- },
- status=HTTP_500_INTERNAL_SERVER_ERROR,
- )
+ email = request.data.get("email") # type: ignore[assignment]
+ if not email:
+ return Response(
+ data={"error": "emailRequired"},
+ status=status.HTTP_400_BAD_REQUEST,
+ )
+
+ associated_user = User.objects.filter(email=email).first()
+
+ # Do not leak whether the user exists / is local; match PasswordResetView semantics.
+ if not (associated_user and associated_user.is_local):
+ return Response(status=HTTP_202_ACCEPTED)
+
+ if not (EMAIL_HOST or EMAIL_HOST_RESCUE):
+ return Response(
+ data={
+ "error": "Email server not configured, please contact your administrator"
+ },
+ status=HTTP_500_INTERNAL_SERVER_ERROR,
+ )
+
+ try:
+ associated_user.mailing(
+ email_template_name="registration/first_connection_email.html",
+ subject=_("CISO Assistant: Invitation"),
+ )
+ logger.info("invitation email sent", email=email, user_id=associated_user.id)
+ except Exception as e:
+ logger.error("invitation email failed", email=email, user_id=associated_user.id, error=e)
+ return Response(
+ data={"error": "An error occurred while sending the email"},
+ status=HTTP_500_INTERNAL_SERVER_ERROR,
+ )
+
+ return Response(status=HTTP_202_ACCEPTED)Also consider tightening permission_classes (e.g. requiring authentication/role) if you don’t intend this endpoint to be publicly callable.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/iam/views.py around lines 270-298, the SendInvtationView has
input-validation, status-code, logging and naming issues: change
request.data["email"] to request.data.get("email") and return
HTTP_400_BAD_REQUEST when the email value is missing/invalid; do not return
HTTP_500 when no user is found — instead treat unknown or non-local users the
same as successful requests and return HTTP_202_ACCEPTED to avoid email
enumeration; replace print(...) with the module logger
(logger.info/logger.error) for sending and exception logging; rename the class
to SendInvitationView and update all imports/URL mappings accordingly; finally
review permission_classes and tighten authentication/roles if the endpoint
should not be public.
| function modalSendInvitation(id: string, email: string, action: string): void { | ||
| const modalComponent: ModalComponent = { | ||
| ref: ConfirmModal, | ||
| props: { | ||
| _form: { id: id, urlmodel: getModelInfo('representatives').urlModel, email: email }, | ||
| id: id, | ||
| debug: false, | ||
| URLModel: getModelInfo('representatives').urlModel, | ||
| formAction: action | ||
| } | ||
| }; | ||
| const modal: ModalSettings = { | ||
| type: 'component', | ||
| component: modalComponent, | ||
| title: m.confirmModalTitle(), | ||
| body: `Do you want to send the invitation to ${email}?` | ||
| }; | ||
| modalStore.trigger(modal); | ||
| } |
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.
Use i18n message instead of hard‑coded confirmation text
The confirmation body text (Do you want to send the invitation to ${email}?) is hard‑coded in English, unlike the rest of the DetailView which uses m.*/safeTranslate. Please switch this to a paraglide message (e.g. m.sendInvitationConfirm({ email })) and add the corresponding translation entry.
🤖 Prompt for AI Agents
In frontend/src/lib/components/DetailView/DetailView.svelte around lines 230 to
248, the modal confirmation body is hard-coded in English; replace the template
string `Do you want to send the invitation to ${email}?` with the paraglide/i18n
message call (e.g. use m.sendInvitationConfirm({ email }) or the project’s
safeTranslate helper) so it uses localized text, and add the corresponding
translation entry (sendInvitationConfirm with an {email} interpolation) to the
app’s translations/messages file(s) for all supported locales.
| {#if displayEditButton()} | ||
| {#if data.urlModel === 'representatives' && data.data.user} | ||
| <button | ||
| class="btn preset-filled-ghost-500 mr-2" | ||
| onclick={() => | ||
| modalSendInvitation( | ||
| data.data.id, | ||
| data.data.email, | ||
| '/representatives/send-invitation' | ||
| )} | ||
| data-testid="send-invitation-button" | ||
| > | ||
| <i class="fa-solid fa-envelope mr-2"></i> | ||
| Send invitation | ||
| </button> |
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.
Localize the “Send invitation” button label
The button label Send invitation is hard‑coded. For consistency with the rest of the UI and to keep translations working, this should come from m.* (e.g. {m.sendInvitation()}) with the corresponding message defined in your messages file.
🤖 Prompt for AI Agents
In frontend/src/lib/components/DetailView/DetailView.svelte around lines 654 to
668 the "Send invitation" button label is hard-coded; replace the literal text
with the localized message call (e.g. use m.sendInvitation() or the appropriate
m.* accessor used in this project) and add the corresponding key/value entry for
sendInvitation in the messages file so translations pick it up; ensure the
component imports/has access to m and update any tests referencing the button
text if necessary to use the localized output or data-testid instead.
| export const POST: RequestHandler = async (event) => { | ||
| const requestInitOptions: RequestInit = { | ||
| method: 'POST' | ||
| }; | ||
| const form = await event.request.formData(); | ||
| const raw = form.get('__superform_json') as string; | ||
|
|
||
| let parsed = JSON.parse(raw); | ||
|
|
||
| let email: string | undefined; | ||
|
|
||
| if (Array.isArray(parsed)) { | ||
| const mapping = parsed[0]; | ||
| if (mapping && typeof mapping === 'object' && typeof mapping.email === 'number') { | ||
| const emailIndex = mapping.email; | ||
| email = parsed[emailIndex]; | ||
| } else { | ||
| email = parsed.find((v: any) => typeof v === 'string' && v.includes('@')); | ||
| } | ||
| } else if (parsed && typeof parsed === 'object') { | ||
| email = parsed.email ?? parsed['__email'] ?? undefined; | ||
| } | ||
|
|
||
| const res = await fetch(`${BASE_API_URL}/iam/send-invitation/`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ email: email }) | ||
| }); | ||
| return new Response(null, { | ||
| headers: { | ||
| 'Content-Type': 'application/json' | ||
| } | ||
| }); |
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.
Validate form payload, ensure email is present, and propagate backend response
The handler is quite optimistic right now:
form.get('__superform_json')andJSON.parse(raw)can fail; no 4xx handling, so you’ll just get a 500 on bad input.emailcan stayundefined, leading to{}being sent to the backend and causing an error there.- The downstream response (
res) is ignored; the client always gets200with an empty body, so messages like “no mailer configured” or other errors are lost.
A more robust version could look like:
export const POST: RequestHandler = async (event) => {
- const requestInitOptions: RequestInit = {
- method: 'POST'
- };
- const form = await event.request.formData();
- const raw = form.get('__superform_json') as string;
-
- let parsed = JSON.parse(raw);
-
- let email: string | undefined;
+ const form = await event.request.formData();
+ const raw = form.get('__superform_json');
+ if (typeof raw !== 'string') {
+ return new Response(JSON.stringify({ error: 'invalidFormPayload' }), {
+ status: 400,
+ headers: { 'Content-Type': 'application/json' }
+ });
+ }
+
+ let parsed: unknown;
+ try {
+ parsed = JSON.parse(raw);
+ } catch {
+ return new Response(JSON.stringify({ error: 'invalidJsonPayload' }), {
+ status: 400,
+ headers: { 'Content-Type': 'application/json' }
+ });
+ }
+
+ let email: string | undefined;
- if (Array.isArray(parsed)) {
+ if (Array.isArray(parsed)) {
const mapping = parsed[0];
if (mapping && typeof mapping === 'object' && typeof mapping.email === 'number') {
const emailIndex = mapping.email;
email = parsed[emailIndex];
} else {
email = parsed.find((v: any) => typeof v === 'string' && v.includes('@'));
}
- } else if (parsed && typeof parsed === 'object') {
+ } else if (parsed && typeof parsed === 'object') {
email = parsed.email ?? parsed['__email'] ?? undefined;
}
- const res = await fetch(`${BASE_API_URL}/iam/send-invitation/`, {
+ if (!email) {
+ return new Response(JSON.stringify({ error: 'emailRequired' }), {
+ status: 400,
+ headers: { 'Content-Type': 'application/json' }
+ });
+ }
+
+ const res = await fetch(`${BASE_API_URL}/iam/send-invitation/`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ email: email })
});
- return new Response(null, {
- headers: {
- 'Content-Type': 'application/json'
- }
- });
+
+ const text = await res.text();
+ return new Response(text || null, {
+ status: res.status,
+ headers: {
+ 'Content-Type': res.headers.get('Content-Type') ?? 'application/json'
+ }
+ });
};This keeps the email extraction logic but fails fast on bad form/JSON, guarantees an email before calling the API, and forwards the backend’s status and body so the UI can react appropriately.
📝 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.
| export const POST: RequestHandler = async (event) => { | |
| const requestInitOptions: RequestInit = { | |
| method: 'POST' | |
| }; | |
| const form = await event.request.formData(); | |
| const raw = form.get('__superform_json') as string; | |
| let parsed = JSON.parse(raw); | |
| let email: string | undefined; | |
| if (Array.isArray(parsed)) { | |
| const mapping = parsed[0]; | |
| if (mapping && typeof mapping === 'object' && typeof mapping.email === 'number') { | |
| const emailIndex = mapping.email; | |
| email = parsed[emailIndex]; | |
| } else { | |
| email = parsed.find((v: any) => typeof v === 'string' && v.includes('@')); | |
| } | |
| } else if (parsed && typeof parsed === 'object') { | |
| email = parsed.email ?? parsed['__email'] ?? undefined; | |
| } | |
| const res = await fetch(`${BASE_API_URL}/iam/send-invitation/`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ email: email }) | |
| }); | |
| return new Response(null, { | |
| headers: { | |
| 'Content-Type': 'application/json' | |
| } | |
| }); | |
| export const POST: RequestHandler = async (event) => { | |
| const form = await event.request.formData(); | |
| const raw = form.get('__superform_json'); | |
| if (typeof raw !== 'string') { | |
| return new Response(JSON.stringify({ error: 'invalidFormPayload' }), { | |
| status: 400, | |
| headers: { 'Content-Type': 'application/json' } | |
| }); | |
| } | |
| let parsed: unknown; | |
| try { | |
| parsed = JSON.parse(raw); | |
| } catch { | |
| return new Response(JSON.stringify({ error: 'invalidJsonPayload' }), { | |
| status: 400, | |
| headers: { 'Content-Type': 'application/json' } | |
| }); | |
| } | |
| let email: string | undefined; | |
| if (Array.isArray(parsed)) { | |
| const mapping = parsed[0]; | |
| if (mapping && typeof mapping === 'object' && typeof mapping.email === 'number') { | |
| const emailIndex = mapping.email; | |
| email = parsed[emailIndex]; | |
| } else { | |
| email = parsed.find((v: any) => typeof v === 'string' && v.includes('@')); | |
| } | |
| } else if (parsed && typeof parsed === 'object') { | |
| email = parsed.email ?? parsed['__email'] ?? undefined; | |
| } | |
| if (!email) { | |
| return new Response(JSON.stringify({ error: 'emailRequired' }), { | |
| status: 400, | |
| headers: { 'Content-Type': 'application/json' } | |
| }); | |
| } | |
| const res = await fetch(`${BASE_API_URL}/iam/send-invitation/`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ email: email }) | |
| }); | |
| const text = await res.text(); | |
| return new Response(text || null, { | |
| status: res.status, | |
| headers: { | |
| 'Content-Type': res.headers.get('Content-Type') ?? 'application/json' | |
| } | |
| }); | |
| }; |
Summary by CodeRabbit