feat: Organisation Invite Flow and SMTP setup#176
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
3 times, most recently
from
f2680ae to
c06410e
Compare
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
2 times, most recently
from
c45cca5 to
7f1fd64
Compare
There was a problem hiding this comment.
How do you invite someone? Add user api does it?
Can you make a different API. Ideally we should have a prediction for users already present in the org. (In application)
There was a problem hiding this comment.
-
Moved that to POST /invite
It shows a modal with the people who are in org and can be given access to the application.
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
from
7f1fd64 to
b7a54b6
Compare
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA comprehensive invitation system is introduced, enabling users to invite organization and application members via email. The system includes database schema for invitations, SMTP email infrastructure with templating, API endpoints for creating/accepting/revoking invites, and frontend pages/components to handle invite flows in login, registration, and user management contexts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant Keycloak
participant Database
participant EmailService
User->>Frontend: Clicks "Invite User" in org
Frontend->>Frontend: Opens OrganizationAccessModal
User->>Frontend: Enters email & selects apps/roles
Frontend->>Backend: POST /organisations/user/invite<br/>{email, org_role, applications}
Backend->>Database: Check for existing pending invite
alt Existing invite found
Backend->>Database: Update token & apps
else No existing invite
Backend->>Database: Create new invite record
end
Backend->>Backend: Generate secure token
Backend->>EmailService: Send invitation email<br/>with token + accept link
Backend->>Frontend: Return {success, operation}
Frontend->>User: Show toast confirmation
rect rgb(200, 220, 255)
Note over User,EmailService: User Accepts Invitation
User->>EmailService: Receives invite email
User->>Frontend: Clicks accept link
Frontend->>Frontend: Validates token, shows invite details
User->>Frontend: Clicks Accept button
Frontend->>Backend: POST /organisations/user/invite/{id}/accept<br/>{token}
Backend->>Database: Verify invite & token not expired
Backend->>Keycloak: Create/link user account
Backend->>Keycloak: Add user to org group
Backend->>Keycloak: Assign org role
Backend->>Keycloak: Assign app roles
Backend->>Database: Update invite status to Accepted
Backend->>Frontend: Return {success, organization, role}
Frontend->>User: Show success, redirect to /dashboard
end
sequenceDiagram
participant UnauthUser
participant LoginPage
participant Frontend
participant Backend
UnauthUser->>LoginPage: Click invite link in email<br/>?invite_token=xyz&email=user@example.com
LoginPage->>Frontend: Extract URL params
Frontend->>Backend: POST /organisation/user/invite/validate<br/>{token}
Backend->>Frontend: Return {invite_id, valid, organization,<br/>role, email, status}
Frontend->>LoginPage: Pre-fill email field (read-only)
Frontend->>LoginPage: Show "Join {org}" banner
LoginPage->>UnauthUser: Display login form with context
UnauthUser->>LoginPage: Enter password, submit
LoginPage->>Frontend: OAuth redirect with invite_token<br/>in localStorage
Frontend->>Backend: Complete OAuth flow
Backend->>Frontend: Return user token
Frontend->>Backend: POST /organisations/user/invite/{id}/accept<br/>{token}
Backend->>Frontend: Confirm acceptance
Frontend->>UnauthUser: Redirect to /dashboard/<br/>as org member
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
from
136a66f to
1ef7184
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Fix all issues with AI agents
In @airborne_dashboard/app/dashboard/[orgId]/[appId]/users/page.tsx:
- Around line 60-92: handleApplicationInvite currently uses Promise.all which
fails fast and hides partial successes; change it to use Promise.allSettled on
the array of apiFetch promises, then iterate the settled results to separate
fulfilled vs rejected responses, count successes and failures, and surface a
summary to the user via toastSuccess/toastError (e.g., "X grants succeeded, Y
failed" and include error details for failures). Ensure you still call
setIsAppAccessModalOpen(false), mutate(), and updateOrgs() after handling the
settled results (but only rely on mutate/updateOrgs for the successful grants),
and log or collect individual error messages from rejected results to include in
the error toast or console.error for debugging.
In @airborne_dashboard/app/dashboard/[orgId]/users/page.tsx:
- Around line 68-72: The addUser, updateUser, removeUser, and transferOwnership
functions lack error handling; wrap each API call (apiFetch in addUser and the
corresponding calls in updateUser/removeUser/transferOwnership) in a try/catch,
only call mutateUsers() and updateOrgs() after a successful await, and in the
catch block log the error and surface it to the UI (e.g., return or set an
error/toast) so failures don’t leave the UI inconsistent; reference the existing
functions addUser, updateUser, removeUser, transferOwnership, apiFetch,
mutateUsers, and updateOrgs when making these changes.
In @airborne_dashboard/app/invitation/[inviteToken]/page.tsx:
- Around line 177-197: The InviteStatus union includes "already_accepted" and
the page component renders UI for inviteStatus but nothing ever sets
inviteStatus to "already_accepted"; either implement detection or remove the
dead branch. Fix option A: in the invite fetching/validation flow (the code that
sets the inviteStatus state in invitation/[inviteToken]/page.tsx), inspect the
API/DB response for an accepted flag or whether the current user is already a
member (e.g., response.invite.accepted or response.user.isMember) and set
inviteStatus = "already_accepted" when true so the existing UI block is
reachable. Fix option B: if no such detection is possible or desired, remove
"already_accepted" from the InviteStatus type and delete the corresponding JSX
branch (the Card UI guarded by inviteStatus === "already_accepted") to eliminate
dead code.
- Around line 70-78: The code checks error.status === 404 which is unreliable
because native Error objects have no status; update the conditional in the
invite handling logic (the block that calls setInviteStatus) to either read the
HTTP status off your API client error wrapper (e.g., error.response?.status or
the typed error your fetch utility throws) or fall back to checking
error.message for a 404 indicator, or change the API utility to attach a status
property to thrown errors; adjust the conditions around
setInviteStatus("invalid")/("expired") accordingly so the 404 case is detected
correctly.
In @airborne_dashboard/app/invitation/not-found.tsx:
- Around line 73-75: The Link in the not-found UI uses an inconsistent support
email (support@airborne.com); update the mailto href and visible text in the
Link component to use the standardized support address used elsewhere (e.g.,
superposition@juspay.in or support@juspay.in) so the contact email is consistent
across the app; locate the Link element with text "Contact support" in the
not-found component and replace both the href value and the displayed email if
present to match the app's canonical support email.
In @airborne_dashboard/app/register/page.tsx:
- Line 91: The toast message uses the HTML entity ’ which will render
literally; update the toastWarning call (toastWarning) to use a real apostrophe
or Unicode right single quote in the second argument string (e.g., "Passwords
don't match" or "Passwords don’t match") so the toast displays correctly.
In @airborne_dashboard/components/invite-management.tsx:
- Around line 259-268: The Search icon is absolutely positioned (Search
component with class "absolute left-3 ...") but the Input component lacks left
padding, causing text to overlap the icon; update the Input's className (on the
Input with value={localSearchTerm} and onChange handler) to include left padding
(e.g., add "pl-10" alongside the existing "max-w-sm") so the input text clears
the icon.
- Around line 354-365: The pagination range logic renders "Showing 1-0 of 0
invitations" when pagination.total_items is 0; update the JSX in
invite-management.tsx inside the block that renders based on pagination to
explicitly handle the empty case: if pagination.total_items === 0 render a
simple "Showing 0 invitations" message, otherwise keep the existing calculation
that uses pagination.current_page, pagination.per_page and
pagination.total_items; also keep the non-pagination fallback that shows
invites.length unchanged.
In @airborne_dashboard/components/organization-access-modal.tsx:
- Around line 126-152: The handleSubmit function currently calls await
onSubmit(invite) but has no catch path, so if onSubmit throws the UI gives no
feedback; add a try/catch around the await in handleSubmit to capture errors,
set a component error state (e.g., submitError via setSubmitError) or call your
toast utility to show the error message, ensure you only call onClose() on
success (inside the try), and always reset setIsSubmitting(false) in finally;
update the component to render submitError (or rely on toast) so users see the
failure.
In @airborne_dashboard/components/user-management.tsx:
- Around line 210-213: The DialogTitle is hardcoded to "Invite user to
Organisation"; update the title in the DialogTitle element inside the Invite
dialog component (where DialogContent/DialogHeader/DialogTitle are rendered) to
use the component's entityType prop (or a small mapping) so it dynamically shows
"Invite user to Organisation" for entityType === 'organisation' and "Invite user
to Application" for entityType === 'application' (ensure proper capitalization
and fallback text if entityType is missing).
In @airborne_dashboard/lib/invitation.ts:
- Around line 39-41: The check uses error.status (if (error.status === 404)) but
apiFetch throws standard Errors without a status, so update handling to either
detect a 404 from the thrown error message or use a custom error type from
apiFetch: Option A — change the condition to inspect the error message (e.g., if
(error.message && error.message.includes('404')) throw new Error("Invalid invite
token"); Option B — modify apiFetch to throw a custom ApiFetchError with a
status property and then change the handler to check (error instanceof
ApiFetchError && error.status === 404) before throwing "Invalid invite token".
Ensure you reference the existing error check (if (error.status === 404)) and
apiFetch when making the change.
In @airborne_server/Cargo.toml:
- Line 37: The Cargo.toml entry for the lettre dependency only enables the
"builder" feature which disables defaults but the code uses
SmtpTransport::starttls_relay(), requiring the smtp-transport feature and a TLS
backend; update the lettre dependency to include "smtp-transport" and a TLS
backend (for example add "native-tls") or set default-features = true so
SmtpTransport and STARTTLS support are available.
In @airborne_server/migrations/20251030123645_create_org_invites_table/up.sql:
- Line 10: Add a UNIQUE constraint on the token column in the
organisation_invites table and ensure an index exists for fast lookups: alter
the CREATE TABLE/column definition for organisation_invites to declare token as
UNIQUE (or add a separate ALTER TABLE ... ADD CONSTRAINT ... UNIQUE on token)
and also create an index if desired (e.g., organisation_invites token index) so
lookups are efficient and collisions are prevented; update the up.sql migration
to include these changes referencing the token column and organisation_invites
table.
In @airborne_server/src/organisation/user/invite.rs:
- Around line 379-394: The debug/info logs leak PII by including user_email;
change logging to avoid raw email by using a non-PII identifier (e.g.,
invite.id, user id, or a truncated/hashed email) and update the log messages in
the add flow around update_invite_status and add_user_with_transaction to
reference that identifier instead of user_email; modify the debug line "Adding
user ..." and the info line in the error branch to remove user_email and include
invite.id or org_id and the error, and make the same replacements for the
similar logging block around lines 436-450.
- Around line 436-450: Logs include PII (user_email); remove it from the
acceptance/decline log statements in this block and instead log non-PII
identifiers such as invite.id or invite.org_id (or a masked value). Locate the
branch that calls update_invite_status and uses InviteStatus::Declined and the
corresponding acceptance branch, and replace info!/debug! messages that
interpolate user_email with messages that reference invite.id or invite.org_id
only.
- Around line 97-106: The code silently swallows serialization errors by calling
serde_json::to_value(apps).unwrap_or_default() when updating
organisation_invites::applications; instead, explicitly serialize and propagate
or handle the error before the diesel update (e.g., let apps_json =
serde_json::to_value(apps)? or map the serde error into your function's error
type) and then use .eq(apps_json) in the update; make the same change for the
other occurrence noted at line 130 so both uses of unwrap_or_default are
replaced with explicit error-handling around serde_json::to_value.
- Around line 724-729: The current match on mail.send().await simply logs at
info! and swallows errors; update the match so failures are surfaced: either (A)
propagate the error from the enclosing function by replacing the match with
mail.send().await.map_err(|e| /* wrap into an appropriate error type and return
*/)? so the invite operation returns Err on email failure, or (B) keep the
operation but increase visibility and include status in the response by changing
info! to warn! or error! for the Err branch and set a field on the outgoing
response (e.g., response.email_sent = false or add email_status) so the caller
knows delivery failed; modify the block around mail.send().await and the
response construction accordingly (refer to mail.send().await, the match block,
the info! macro, and the response variable/InviteResponse used later).
In @airborne_server/src/utils/db/schema.rs:
- Around line 81-96: The organisation_invites table schema misses a uniqueness
constraint and index on the token column, causing full table scans in
find_invite_by_token(); update the migration that creates the
hyperotaserver.organisation_invites table to add a unique index on the token
column (organisation_invites.token) so lookups are indexed and tokens are
enforced unique, and rerun or add an idempotent migration that creates the index
if not exists to avoid duplicate migrations while preserving the existing
created_at-based expiry logic referenced by find_invite_by_token().
In @airborne_server/templates/org_invitation.html:
- Around line 59-64: The template hardcodes "7 days" in org_invitation.html
which can diverge from backend logic; replace the literal with a template
variable like {{ invitation_expiration_days }} (or {{ expiration_days }}) in the
paragraph and ensure the backend rendering function that sends or renders this
template (e.g., send_invitation_email / render_invitation) passes that variable
(derived from the actual expiration logic) into the template context so the
displayed period always matches the backend.
🧹 Nitpick comments (26)
airborne_dashboard/app/oauth/callback/page.tsx (1)
75-79: Remove debug console.log before merging.The
console.logon line 77 appears to be debug output that should be removed for production code.Also, the type cast on line 75 (
res = res as OAuthUserResponse) doesn't actually narrow the type ofres- it just reassigns it. Consider using a properly typed variable instead.Suggested approach
- res = res as OAuthUserResponse; - - console.log("token exchange", oauthAction, res); - setToken(res.user_token?.access_token || ""); - setUser({ user_id: res.user_id, name: res.username, is_super_admin: res.is_super_admin }); // OAuth users will get name from API response + const userRes = res as OAuthUserResponse; + setToken(userRes.user_token?.access_token || ""); + setUser({ user_id: userRes.user_id, name: userRes.username, is_super_admin: userRes.is_super_admin });airborne_server/templates/org_invitation.txt (1)
14-14: Consider parameterizing the expiry duration.The "7 days" expiry is hardcoded in the template. If the expiry duration is configurable elsewhere, consider passing it as a template variable (e.g.,
{{ expiry_days }}) to keep the template in sync with the actual expiry logic.airborne_server/templates/org_invitation.html (1)
74-76: Hardcoded URLs may cause issues in non-production environments.The footer links point to production URLs (
airborne.juspay.in). Consider templating these if the application is deployed to multiple environments.airborne_dashboard/app/invitation/not-found.tsx (1)
3-3: Unused import.The
Reacttype import is not used in this file.Remove unused import
-import type React from "react";airborne_server/migrations/20251030123645_create_org_invites_table/up.sql (1)
4-13: Consider addingexpires_atcolumn and index onFor invite expiration handling, an
expires_attimestamp column would be more flexible than status-based expiry logic. Additionally, an index onSuggested additions
token TEXT NOT NULL, status hyperotaserver.invite_status NOT NULL DEFAULT 'pending', - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + expires_at TIMESTAMPTZ NOT NULL DEFAULT (NOW() + INTERVAL '7 days') ); CREATE INDEX IF NOT EXISTS organisation_invites_org_id_idx ON hyperotaserver.organisation_invites (org_id); +CREATE INDEX IF NOT EXISTS organisation_invites_email_idx ON hyperotaserver.organisation_invites (email);airborne_dashboard/app/register/page.tsx (2)
49-86: MovefetchInviteDetailsoutsideuseEffectand add missing dependencies.Defining
fetchInviteDetailsinsideuseEffectis valid but therouterdependency is missing from the dependency array. Additionally, there's no cleanup for the async operation which could cause state updates on unmounted components.Suggested improvements
+ const fetchInviteDetails = async (inviteTokenParam: string) => { + setIsLoadingInvite(true); + try { + const details = await validateInviteToken(inviteTokenParam); + setInviteInfo({ + organization: details.organization, + role: details.role, + email: details.email, + }); + setFormData((prev) => ({ + ...prev, + email: details.email, + })); + } catch (error) { + console.error("Failed to fetch invite details:", error); + } finally { + setIsLoadingInvite(false); + } + }; + useEffect(() => { if (token) router.replace("/dashboard"); const inviteTokenParam = searchParams?.get("invite_token"); const redirectToParam = searchParams?.get("redirect_to"); if (inviteTokenParam) { setInviteToken(inviteTokenParam); setRedirectTo(redirectToParam); fetchInviteDetails(inviteTokenParam); } - - async function fetchInviteDetails(token: string) { - // ... function body - } - }, [token, searchParams]); + }, [token, searchParams, router]);
339-343: Email field disabled logic may have edge case issues.The condition
disabled={inviteInfo?.email === formData.email}could be bypassed if the user clears the email field first, or could incorrectly disable the field if the user's intended email happens to match. Consider using a dedicated flag.Suggested fix
+ const [isEmailFromInvite, setIsEmailFromInvite] = useState(false); // In fetchInviteDetails, after setting email: + setIsEmailFromInvite(true); // In the Input: - disabled={inviteInfo?.email === formData.email} + disabled={isEmailFromInvite}airborne_dashboard/app/login/page.tsx (2)
213-216: RedundantreadOnlyanddisabledattributes on input.Using both
readOnlyanddisabledis redundant.disabledalone is sufficient and provides the correct visual feedback.Suggested fix
- readOnly={!!inviteEmail} - disabled={!!inviteEmail} + disabled={!!inviteEmail}
85-92: Clean up localStorage items after OAuth callback completes.The
oauthInviteTokenandoauthRedirectTolocalStorage items are set but never removed after use. This could cause stale data issues on subsequent logins.Consider adding cleanup logic in the OAuth callback handler or when the component mounts with an existing token.
airborne_dashboard/app/dashboard/[orgId]/users/page.tsx (2)
36-36:inviteLimitdoesn't need to be state.Since
inviteLimitis never updated viasetInviteLimit(which isn't even destructured), it can be a simple constant.Suggested fix
- const [inviteLimit] = useState(10); + const inviteLimit = 10;
102-106: Comment mentions debouncing but no debounce is implemented.The comment says "Handle invitation search with debouncing" but the function directly updates state without any debounce logic. This could cause excessive API calls on rapid typing.
Suggested implementation with debounce
+import { useDebouncedCallback } from 'use-debounce'; - // Handle invitation search with debouncing - const handleInviteSearchChange = (search: string) => { - setInviteSearchTerm(search); - setInvitePage(1); // Reset to first page when searching - }; + // Handle invitation search with debouncing + const handleInviteSearchChange = useDebouncedCallback((search: string) => { + setInviteSearchTerm(search); + setInvitePage(1); // Reset to first page when searching + }, 300);Alternatively, if you don't want to add a dependency, implement with
useRefandsetTimeout.airborne_server/src/main.rs (2)
188-191: Uselog::error!instead ofprintln!for consistency.Template parsing errors should use the logging framework for consistency with the rest of the application and proper log aggregation.
Suggested fix
let tera = Tera::new("templates/**/*").map_err(|e| { - println!("Parsing error(s): {}", e); + log::error!("Template parsing error(s): {}", e); std::io::Error::other("Template parsing error") })?;
169-186: Consider verifying SMTP connection at startup.The SMTP transport is built but not tested until the first email is sent. A misconfigured SMTP server won't be detected until runtime. Consider adding a connection test during startup.
Suggested addition after mailer creation
// After building the mailer, test the connection if !mock_email_sending { use lettre::Transport; if let Err(e) = mailer.test_connection() { log::warn!("SMTP connection test failed: {}. Emails may not be sent.", e); } else { info!("SMTP connection verified successfully"); } }airborne_dashboard/components/application-access-modal.tsx (2)
106-123: Add user feedback when submission fails.Currently, if
onSubmitthrows an error, the modal stays open but there's no visual feedback to the user about what went wrong. The error is silently caught by thefinallyblock.Suggested fix
+import { toastError } from "@/hooks/use-toast"; const handleSubmit = async () => { const invites = Object.entries(selectedUsers).map(([userId, role]) => ({ userId, role, })); if (invites.length === 0) { return; } setIsSubmitting(true); try { await onSubmit(invites); onClose(); - } finally { + } catch (error: any) { + toastError("Failed to Grant Access", error.message || "An error occurred while granting access"); + } finally { setIsSubmitting(false); } };
29-32: ExportApplicationAccessInviteinterface for external use.The interface is defined but not exported. If consumers of this component need to type the callback parameter, they'll need access to this type.
Suggested fix
-interface ApplicationAccessInvite { +export interface ApplicationAccessInvite { userId: string; role: string; }airborne_server/src/utils/mail.rs (2)
50-61: Hardcoded sender email should be configurable.The "from" email address
no-reply@airborne.iois hardcoded. Consider making this configurable via environment variable or AppState to support different deployment environments.♻️ Suggested approach
pub struct Mail<'a> { pub smtp_transport: &'a Arc<SmtpTransport>, pub tera: &'a Arc<Tera>, pub context: tera::Context, pub to: String, pub subject: String, pub text_body_template: String, pub html_body_template: Option<String>, + pub from_email: String, + pub from_name: String, }Then use the configurable values in the builder instead of the hardcoded string.
9-17: Consider making struct fields private with a builder pattern.All fields are
pub, allowing direct mutation after construction. For a mail utility, a builder pattern or private fields with the constructor as the only entry point would provide better encapsulation and prevent misuse.airborne_dashboard/components/invite-management.tsx (1)
158-169: Potential stale closure and missing dependency warning.The second
useEffect(Lines 165-169) intentionally omitslocalSearchTermfrom dependencies to avoid loops, but this will trigger ESLintreact-hooks/exhaustive-depswarnings. Consider using a ref to track whether the change originated from props vs local input.♻️ Alternative approach using ref
const isExternalUpdate = React.useRef(false); React.useEffect(() => { if (debouncedSearchTerm !== searchTerm && !isExternalUpdate.current) { onSearchChange?.(debouncedSearchTerm); } isExternalUpdate.current = false; }, [debouncedSearchTerm, onSearchChange, searchTerm]); React.useEffect(() => { if (searchTerm !== localSearchTerm) { isExternalUpdate.current = true; setLocalSearchTerm(searchTerm); } }, [searchTerm, localSearchTerm]);airborne_dashboard/components/organization-access-modal.tsx (1)
71-81: Email validation regex may allow invalid emails.The regex
/^[^\s@]+@[^\s@]+\.[^\s@]+$/is permissive and allows some technically invalid emails. Consider using a more robust validation or the browser's built-in validation viatype="email"(which is already set on the input).airborne_dashboard/app/dashboard/[orgId]/[appId]/users/page.tsx (1)
99-105: Unnecessary Fragment wrapper in loading state.The empty fragment
<></>wrapper around the loading div is unnecessary.♻️ Simplified loading state
if (isLoading) { return ( - <> - <div className="p-6">Loading users...</div> - </> + <div className="p-6">Loading users...</div> ); }airborne_dashboard/app/invitation/[inviteToken]/page.tsx (3)
45-53: useEffect may cause unnecessary re-validations.The
validateInviteTokenfunction is not memoized withuseCallback, so every render creates a new function reference. While this doesn't directly affect the useEffect (since the function isn't in deps), the effect runs on everytokenoruserchange, which could cause redundant API calls during auth state changes.♻️ Suggested improvement
+ const validateInviteToken = React.useCallback(async () => { // ... existing implementation + }, [inviteToken, token, user]); useEffect(() => { if (!inviteToken) { setInviteStatus("invalid"); return; } validateInviteToken(); - }, [inviteToken, token, user]); + }, [inviteToken, validateInviteToken]);
336-352: Email in URL query parameter may be a privacy/security concern.The login URL includes the invite email as a query parameter. This could be logged in server access logs, browser history, or shared inadvertently. Consider passing only the invite token and letting the login page fetch the email if needed.
♻️ Remove email from URL
<Button asChild className="flex-1 h-11"> <Link - href={`/login?invite_token=${inviteToken}&email=${encodeURIComponent(inviteDetails?.email || "")}&redirect_to=${encodeURIComponent(`/invitation/${inviteToken}`)}`} + href={`/login?invite_token=${inviteToken}&redirect_to=${encodeURIComponent(`/invitation/${inviteToken}`)}`} > Sign In </Link> </Button>The login page can pre-fill the email by validating the invite token if needed.
59-63: Clarify field semantics:user.namefield appears to store email/username but naming is misleading.Line 60 compares
details.emailwithuser.name. While the current implementation treatsuser.nameas an email or username (populated from login form input or OAuthusernamefield), the field name is ambiguous. This comparison will break if the backend API changes whatuser.namerepresents or if it's used inconsistently. Consider either:
- Renaming the field to
user.emailoruser.usernameto clarify its purpose, or- Verifying and documenting that
user.nameis guaranteed to always contain an email addressairborne_dashboard/types/invitation.ts (2)
22-36: Consider consolidating duplicate response types.
AcceptInviteResponseandDeclineInviteResponseare structurally identical. Consider using a singleInviteRSVPResponsetype (matching the backend naming) or a type alias to reduce duplication.♻️ Suggested consolidation
-export interface AcceptInviteResponse { - success: boolean; - message: string; - organization: string; - role: string; - action: "Accepted" | "Declined"; -} - -export interface DeclineInviteResponse { - success: boolean; - message: string; - organization: string; - role: string; - action: "Accepted" | "Declined"; -} +export interface InviteRSVPResponse { + success: boolean; + message: string; + organization: string; + role: string; + action: "Accepted" | "Declined"; +} + +export type AcceptInviteResponse = InviteRSVPResponse; +export type DeclineInviteResponse = InviteRSVPResponse;
1-9: Consider reusingValidateInviteResponseforInviteDetails.
InviteDetailsis a subset ofValidateInviteResponse(missing onlyvalid: boolean). You could derive one from the other to maintain consistency.♻️ Suggested approach
export interface ValidateInviteResponse { invite_id: string; valid: boolean; email: string; organization: string; role: string; status: string; created_at: string; inviter?: string; } export type InviteDetails = Omit<ValidateInviteResponse, 'valid'>;airborne_dashboard/lib/invitation.ts (1)
10-10: Remove unused parameter_authToken.The
_authTokenparameter is declared but never used in the function body. SincerequireAuth: falseis set, authentication is intentionally skipped.♻️ Suggested fix
-export async function validateInviteToken(token: string, _authToken?: string): Promise<InviteDetails> { +export async function validateInviteToken(token: string): Promise<InviteDetails> {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
API_DOCUMENTATION.mdairborne_dashboard/app/dashboard/[orgId]/[appId]/users/page.tsxairborne_dashboard/app/dashboard/[orgId]/users/page.tsxairborne_dashboard/app/invitation/[inviteToken]/page.tsxairborne_dashboard/app/invitation/not-found.tsxairborne_dashboard/app/login/page.tsxairborne_dashboard/app/oauth/callback/page.tsxairborne_dashboard/app/register/page.tsxairborne_dashboard/components/application-access-modal.tsxairborne_dashboard/components/invite-management.tsxairborne_dashboard/components/organization-access-modal.tsxairborne_dashboard/components/user-management.tsxairborne_dashboard/lib/invitation.tsairborne_dashboard/next.config.mjsairborne_dashboard/types/invitation.tsairborne_server/.env.exampleairborne_server/Cargo.tomlairborne_server/localhost:5433airborne_server/migrations/20251030123645_create_org_invites_table/down.sqlairborne_server/migrations/20251030123645_create_org_invites_table/up.sqlairborne_server/src/main.rsairborne_server/src/organisation/application/user.rsairborne_server/src/organisation/application/user/types.rsairborne_server/src/organisation/user.rsairborne_server/src/organisation/user/invite.rsairborne_server/src/organisation/user/invite/types.rsairborne_server/src/organisation/user/types.rsairborne_server/src/types.rsairborne_server/src/utils.rsairborne_server/src/utils/db/models.rsairborne_server/src/utils/db/schema.rsairborne_server/src/utils/mail.rsairborne_server/templates/org_invitation.htmlairborne_server/templates/org_invitation.txt
💤 Files with no reviewable changes (1)
- airborne_server/src/organisation/application/user/types.rs
🧰 Additional context used
🧬 Code graph analysis (14)
airborne_dashboard/app/invitation/not-found.tsx (2)
airborne_dashboard/components/ui/card.tsx (5)
Card(56-56)CardHeader(56-56)CardTitle(56-56)CardDescription(56-56)CardContent(56-56)airborne_dashboard/components/ui/button.tsx (1)
Button(50-50)
airborne_dashboard/app/dashboard/[orgId]/users/page.tsx (10)
airborne_dashboard/components/user-management.tsx (3)
User(21-24)AccessLevel(19-19)UserManagement(84-564)airborne_dashboard/providers/app-context.tsx (1)
useAppContext(152-156)airborne_dashboard/lib/api.ts (1)
apiFetch(19-118)airborne_dashboard/lib/invitation.ts (2)
listInvites(72-91)revokeInvite(94-106)airborne_dashboard/hooks/use-toast.ts (2)
toastSuccess(192-198)toastError(200-206)airborne_dashboard/components/ui/tabs.tsx (4)
Tabs(42-42)TabsList(42-42)TabsTrigger(42-42)TabsContent(42-42)airborne_dashboard/components/ui/card.tsx (5)
Card(56-56)CardContent(56-56)CardHeader(56-56)CardTitle(56-56)CardDescription(56-56)airborne_dashboard/components/ui/button.tsx (1)
Button(50-50)airborne_dashboard/components/invite-management.tsx (1)
InviteManagement(122-448)airborne_dashboard/components/organization-access-modal.tsx (1)
OrganizationAccessModal(47-334)
airborne_dashboard/app/register/page.tsx (4)
airborne_dashboard/lib/invitation.ts (1)
validateInviteToken(10-46)airborne_dashboard/hooks/use-toast.ts (1)
toastWarning(208-214)airborne_dashboard/components/ui/card.tsx (4)
CardTitle(56-56)CardDescription(56-56)CardHeader(56-56)CardContent(56-56)airborne_dashboard/components/ui/alert.tsx (2)
Alert(49-49)AlertDescription(49-49)
airborne_dashboard/components/user-management.tsx (4)
airborne_dashboard/hooks/use-toast.ts (2)
toastSuccess(192-198)toastError(200-206)airborne_dashboard/components/ui/dialog.tsx (2)
DialogTitle(121-121)DialogHeader(118-118)airborne_dashboard/components/ui/input.tsx (1)
Input(21-21)airborne_dashboard/components/ui/button.tsx (1)
Button(50-50)
airborne_dashboard/app/login/page.tsx (4)
airborne_dashboard/components/ui/card.tsx (3)
CardDescription(56-56)CardHeader(56-56)CardContent(56-56)airborne_dashboard/components/ui/label.tsx (1)
Label(21-21)airborne_dashboard/components/ui/input.tsx (1)
Input(21-21)airborne_dashboard/components/ui/tooltip.tsx (2)
TooltipProvider(48-48)Tooltip(48-48)
airborne_server/src/utils/mail.rs (1)
airborne_server/src/types.rs (3)
message(71-73)message(216-219)new(309-315)
airborne_server/src/organisation/application/user.rs (1)
airborne_server/src/organisation/application/user/transaction.rs (2)
get_user_current_role(398-436)add_user_with_transaction(27-79)
airborne_server/src/main.rs (2)
airborne_server/src/utils/mail.rs (1)
new(20-38)airborne_server/src/organisation/user/invite.rs (1)
add_public_routes(54-56)
airborne_server/src/organisation/user/invite/types.rs (1)
airborne_dashboard/types/invitation.ts (3)
InviteListItem(49-55)ListInvitesResponse(39-47)ValidateInviteResponse(11-20)
airborne_dashboard/app/dashboard/[orgId]/[appId]/users/page.tsx (7)
airborne_dashboard/components/user-management.tsx (3)
User(21-24)AccessLevel(19-19)canUpdateUsers(63-73)airborne_dashboard/providers/app-context.tsx (1)
useAppContext(152-156)airborne_dashboard/lib/api.ts (1)
apiFetch(19-118)airborne_dashboard/hooks/use-toast.ts (2)
toastSuccess(192-198)toastError(200-206)airborne_dashboard/components/ui/card.tsx (4)
Card(56-56)CardHeader(56-56)CardTitle(56-56)CardContent(56-56)airborne_dashboard/components/ui/button.tsx (1)
Button(50-50)airborne_dashboard/components/application-access-modal.tsx (1)
ApplicationAccessModal(52-292)
airborne_dashboard/lib/invitation.ts (2)
airborne_dashboard/types/invitation.ts (5)
InviteDetails(1-9)ValidateInviteResponse(11-20)AcceptInviteResponse(22-28)DeclineInviteResponse(30-36)ListInvitesResponse(39-47)airborne_dashboard/lib/api.ts (1)
apiFetch(19-118)
airborne_server/src/organisation/user.rs (2)
airborne_server/src/organisation/user/invite.rs (1)
add_routes(45-52)airborne_server/src/user.rs (1)
add_routes(38-46)
airborne_server/src/organisation/user/invite.rs (4)
airborne_server/src/organisation/application/user.rs (2)
find_application(143-187)req(61-63)airborne_server/src/organisation/user.rs (3)
find_organization(100-120)get_org_context(53-69)req(58-60)airborne_server/src/utils/keycloak.rs (2)
find_org_group(112-135)prepare_user_action(94-110)airborne_server/src/utils/mail.rs (1)
new(20-38)
airborne_dashboard/app/oauth/callback/page.tsx (2)
airborne_dashboard/providers/app-context.tsx (1)
useApp(158-160)airborne_dashboard/lib/api.ts (1)
apiFetch(19-118)
🪛 dotenv-linter (4.0.0)
airborne_server/.env.example
[warning] 47-47: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 51-51: [UnorderedKey] The SMTP_PASSWORD key should go before the SMTP_PORT key
(UnorderedKey)
🪛 GitHub Check: CodeQL
airborne_server/src/organisation/user/invite.rs
[failure] 379-379: Cleartext logging of sensitive information
This operation writes ... .clone() to a log file.
This operation writes ... .clone() to a log file.
[failure] 390-390: Cleartext logging of sensitive information
This operation writes ... .clone() to a log file.
This operation writes ... .clone() to a log file.
[failure] 436-436: Cleartext logging of sensitive information
This operation writes ... .clone() to a log file.
This operation writes ... .clone() to a log file.
[failure] 441-441: Cleartext logging of sensitive information
This operation writes ... .clone() to a log file.
This operation writes ... .clone() to a log file.
[failure] 447-447: Cleartext logging of sensitive information
This operation writes ... .clone() to a log file.
This operation writes ... .clone() to a log file.
🪛 LanguageTool
airborne_server/templates/org_invitation.txt
[uncategorized] ~7-~7: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...2. Monitor performance and analytics in real time 3. Benefit from enterprise-grade securi...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~17-~17: ‘by mistake’ might be wordy. Consider a shorter alternative.
Context: ...}}. If you believe this was sent to you by mistake, you can safely ignore it. Learn more ...
(EN_WORDINESS_PREMIUM_BY_MISTAKE)
API_DOCUMENTATION.md
[uncategorized] ~525-~525: Do not mix variants of the same word (‘organisation’ and ‘organization’) within a single text.
Context: ...te` Description: Adds a user to an organisation with specified access level. If the use...
(EN_WORD_COHERENCY)
[style] ~734-~734: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...pts an organization invitation using an invite token. The authenticated user will be a...
(AN_INVITE)
🔇 Additional comments (43)
airborne_dashboard/next.config.mjs (1)
17-18: LGTM!The addition of the singular
organisationpath segment correctly extends the API rewrite rule to support the new organisation invite endpoints introduced in this PR. The regex syntax is correct, and the change is appropriately scoped to non-production environments only.airborne_server/src/utils.rs (1)
20-20: LGTM!The new
airborne_server/.env.example (1)
46-51: LGTM!The SMTP configuration follows standard conventions. Port 587 (STARTTLS) is the recommended submission port. The inline comment on
MOCK_EMAIL_SENDINGis helpful for developers.airborne_server/Cargo.toml (1)
40-40: LGTM!The
randandteradependencies are appropriate for generating tokens and rendering email templates respectively.Also applies to: 47-47
airborne_dashboard/app/oauth/callback/page.tsx (1)
82-93: Intentional full page reload?Using
window.location.replace()instead ofrouter.replace()triggers a full page reload rather than client-side navigation. This may be intentional to ensure a clean state after OAuth, but it's worth noting the behavior difference.The localStorage cleanup on lines 85-86 looks good for preventing stale redirect data.
airborne_server/src/organisation/application/user.rs (3)
30-35: LGTM!The import restructuring cleanly separates application-level types from organization-level user types, which supports the new invitation flow module organization.
45-46: LGTM!Re-exporting
add_user_with_transactionallows the invitation acceptance flow to reuse this function without duplicating the user-to-application assignment logic.
143-143: LGTM!Making
find_applicationpublic enables reuse in the invitation flow for validating and resolving application contexts during invite acceptance.airborne_server/migrations/20251030123645_create_org_invites_table/down.sql (1)
1-5: LGTM!The down migration correctly reverses the up migration. The
IF EXISTSandCASCADEclauses handle dependency ordering, making the migration idempotent and safe to run.Note: The index drop on line 5 will be a no-op since
CASCADEon line 2 already removes dependent indexes, but this is harmless.airborne_server/templates/org_invitation.txt (1)
1-22: LGTM overall!The plain text email template is well-structured with clear call-to-action and appropriate branding. The Tera placeholders (
{{ organization }},{{ role }},{{ invitation_url }}) align with the expected invitation context.airborne_dashboard/app/invitation/not-found.tsx (1)
10-83: Component implementation looks good.The UI provides clear feedback for invalid invitation scenarios with appropriate CTAs. The light/dark mode logo handling and responsive button layout are well implemented.
airborne_server/src/utils/db/schema.rs (1)
4-12: Custom SQL types are correctly defined.The
InviteRoleandInviteStatustypes properly reference the Postgres schema and will map to the corresponding enums in the models.airborne_server/src/organisation/user/types.rs (3)
22-28: LGTM! Clean mapping between AccessLvl and InviteRole.The
to_invite_rolemethod provides a clear conversion path between the access level types used in user requests and the invite-specific role type used in the database.
31-35: ApplicationAccess struct is well-defined.Simple struct with appropriate fields for specifying per-application access levels.
37-42: Optional applications field maintains backward compatibility.Making
applicationsoptional inUserRequestallows existing callers to continue working without modification.airborne_server/templates/org_invitation.html (1)
54-56: No action needed. Theinvitation_urlis constructed entirely from server-controlled values:public_urlfrom environment configuration andnew_tokengenerated from alphanumeric characters only. Both components are safe and do not require additional URL encoding. Tera's default HTML escaping is sufficient for href attributes.Likely an incorrect or invalid review comment.
airborne_server/src/types.rs (1)
48-49: LGTM! Arc wrapping is appropriate for thread-safe shared state.The
Arc<SmtpTransport>andArc<Tera>additions correctly enable sharing these resources across request handlers. The pattern aligns with lettre's design: SmtpTransport has a built-in connection pool and reuses connections when the transport instance is shared across sends (which Arc ensures). Error handling for SMTP sends is already implemented in the mail module with proper logging and error conversion.airborne_server/src/organisation/user.rs (1)
43-50: LGTM - Route structure properly integrates invite submodule.The invite routes are correctly mounted as a nested scope under
/invite, and the module visibility changes appropriately expose the types and invite functionality needed by other parts of the codebase.airborne_server/src/main.rs (1)
175-177: Consider encrypting SMTP credentials with KMS like other secrets.Other sensitive credentials like
KEYCLOAK_SECRETandSUPERPOSITION_TOKENare decrypted via KMS, but SMTP credentials are read as plaintext environment variables. For consistency and security, consider applying the same encryption pattern.Verify if this is intentional based on your deployment model. If SMTP credentials need the same protection:
- let smtp_user = std::env::var("SMTP_USER").expect("SMTP_USER must be set"); - let smtp_password = std::env::var("SMTP_PASSWORD").expect("SMTP_PASSWORD must be set"); + let enc_smtp_user = std::env::var("SMTP_USER").expect("SMTP_USER must be set"); + let enc_smtp_password = std::env::var("SMTP_PASSWORD").expect("SMTP_PASSWORD must be set"); + let smtp_user = decrypt_kms(&aws_kms_client, enc_smtp_user).await; + let smtp_password = decrypt_kms(&aws_kms_client, enc_smtp_password).await;airborne_dashboard/components/application-access-modal.tsx (1)
21-27: No mismatch betweenOrgUserinterface and data structure.The
OrgUserinterface defines anidfield that is properly populated during data transformation inusers/page.tsxvia the mappingid: user.username. The component receives correctly transformed data where theidfield is set, so the selection logic usinguser.idworks as intended.Likely an incorrect or invalid review comment.
airborne_server/src/utils/mail.rs (1)
40-100: LGTM on the send implementation.The async send logic properly handles template rendering in blocking context, constructs multipart emails correctly when HTML is provided, and maps errors appropriately to
ABErrorvariants. Error logging on send failure is helpful for debugging.airborne_dashboard/components/invite-management.tsx (1)
122-143: Well-structured component with good UX patterns.The component handles loading states, empty states, and error feedback appropriately. The optimistic UI for revoking invites and the debounced search are good UX patterns. Modal integration is clean.
Also applies to: 217-424
airborne_server/src/utils/db/models.rs (3)
16-45: LGTM on InviteRole enum implementation.The ToSql/FromSql implementations correctly handle serialization to/from PostgreSQL custom types with proper error handling for unrecognized variants.
47-81: LGTM on InviteStatus enum implementation.Consistent with InviteRole, properly handles all status variants with appropriate error handling.
267-278: Struct does not requireDeserialize.
OrganisationInviteEntryis only used as a database query result and API response type. It's never deserialized from JSON input—instances are created from Rust code or loaded from the database. TheSerializederive is correct for response serialization;Deserializeis not needed.Likely an incorrect or invalid review comment.
airborne_dashboard/components/user-management.tsx (2)
114-137: Good implementation of invitation flow with proper feedback.The loading state management, toast notifications on success/error, and callback invocation are well-implemented. The finally block ensures
isInvitingis reset even on errors.
244-249: LGTM on button states.Buttons are properly disabled during submission, and text updates to reflect the sending state.
airborne_dashboard/components/organization-access-modal.tsx (2)
154-156: Consider requiring at least one application selection.
canSubmitonly checks email validity, but sending an invite without any application access might not be the intended UX. Consider addingselectedCount > 0to the validation if application access is mandatory.Is it valid to invite a user to an organization without granting them access to any applications?
47-69: Well-structured modal with good UX.The component handles loading states, provides clear visual feedback, and has a clean layout with proper accessibility (labels, form structure). The "Select All" functionality is a nice touch.
Also applies to: 158-333
airborne_dashboard/app/dashboard/[orgId]/[appId]/users/page.tsx (2)
127-190: Good UI structure for application user management.The conditional rendering based on permissions, the info card about available users, and integration with the ApplicationAccessModal are well-implemented.
111-122: Theusernamedespite the API not providing email data.The API endpoint
/organisations/user/listreturns users with onlyusernameandrolesfields. The current code assumes username is email to satisfy theOrgUserinterface requirement. Fetch the actual email from the API or update the data model to not require the email field.airborne_dashboard/app/invitation/[inviteToken]/page.tsx (1)
82-118: Good UX for invitation handling.The page handles multiple states well (loading, error, valid), provides clear feedback, and gracefully handles both authenticated and unauthenticated users. The role descriptions are helpful for users to understand what they're accepting.
Also applies to: 199-370
airborne_server/src/organisation/user/invite/types.rs (1)
1-71: LGTM! Type definitions are well-structured and consistent with frontend.The types align well with the corresponding TypeScript interfaces in
airborne_dashboard/types/invitation.ts. A few observations:
- The type alias
DeclineInviteRequest = AcceptInviteRequestis a clean DRY approachInviteActionenum variants serialize as"Accepted"/"Declined"by default with serde, matching the frontend'saction: "Accepted" | "Declined"union typePaginationInfo.total_itemsusingi64correctly matches the database count return typeairborne_dashboard/lib/invitation.ts (4)
48-70: LGTM on accept/decline invite functions.The authorization header pattern and request structure are consistent with other authenticated endpoints.
72-91: LGTM on listInvites function.Properly passes pagination params via query and disables error toasts for controlled error handling.
108-156: LGTM on sendOrganizationInvite and sendApplicationInvites.Request body transformation from camelCase to snake_case is correctly handled for backend compatibility.
12-12: The path difference is intentional, not an inconsistency. The backend routes are structured with/organisation/user/invite(singular) for public, unauthenticated endpoints and/organisations(plural) for authenticated endpoints. Thevalidateendpoint correctly uses the public path since it's designed to work without authentication (requireAuth: false), while all other endpoints correctly use the authenticated/organisationsscope. No changes needed.Likely an incorrect or invalid review comment.
API_DOCUMENTATION.md (2)
525-598: LGTM! Comprehensive documentation for the invite flow.The documentation clearly explains:
- The dual behavior (existing users added immediately, non-existing users get email invites)
- Operation types (
add,invite_created,invite_updated)- Example responses for different scenarios
The mixed
organisation/organizationspelling noted by static analysis reflects the codebase's British English naming convention for API paths while using American English in descriptions - this is acceptable.
729-990: LGTM! Well-documented invite management endpoints.The new Section 5 provides comprehensive coverage of:
- Accept/List/Revoke invite endpoints with proper schemas
- Query parameters and pagination for list endpoint
- Error cases and HTTP status codes
- Security notes about token generation and expiry
airborne_server/src/organisation/user/invite.rs (4)
139-148: Token generation looks secure.Using
rand::thread_rng()withAlphanumericfor 64 characters provides ~381 bits of entropy (log2(62^64)), which is cryptographically adequate for invite tokens.
304-467: Good transaction-like pattern for invite acceptance.The code properly reverts the invite status back to
Pendingif adding the user to the organization fails. This provides reasonable consistency guarantees.However, note that if application access grants fail (lines 405-434), the user is already added to the org but the invite remains accepted - this is a partial success state. Consider whether this should also trigger a rollback.
Is partial success (user added to org but not all apps) an acceptable state? If not, consider wrapping the entire operation in a more comprehensive rollback mechanism.
45-56: LGTM! Clean route registration.Routes are well-organized with authenticated routes in
add_routes()and the public validation endpoint inadd_public_routes().
510-610: LGTM! Well-implemented list endpoint with proper pagination.The list implementation correctly:
- Validates and clamps pagination parameters
- Supports search filtering on email and role
- Supports status filtering with validation
- Calculates total pages correctly
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
3 times, most recently
from
6cae6a8 to
4e90be3
Compare
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
from
4e90be3 to
9d2b5e6
Compare
yuvrajjsingh0
force-pushed
the
invite-user-to-org
branch
from
5e90faa to
85264cb
Compare
3 participants
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.