-
Notifications
You must be signed in to change notification settings - Fork 32
Feat: Support invitation codes #429
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
WalkthroughA single-file change to the authentication middleware that adds support for handling invitation codes passed as query parameters. When an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (2)
src/authMiddleware/authMiddleware.ts (2)
58-64: Improve error handling for invitation code redirect failures.The catch block only logs the error in debug mode and allows execution to continue. If the redirect fails, the middleware will proceed with the normal authentication flow, which may not be the intended behavior for invitation flows.
Consider either:
- Re-throwing the error to prevent continuing with the normal flow
- Returning a more explicit error response
- Adding a fallback redirect behavior
} catch (error) { if (config.isDebugMode) { console.error( "authMiddleware: error redirecting to register with invitation code", + error, ); } + // Fallback to login page if redirect fails + return NextResponse.redirect( + new URL(loginRedirectUrl, options?.redirectURLBase || config.redirectURL), + ); }
37-65: Consider adding validation or configuration for invitation codes.The current implementation accepts any invitation code value without validation. Depending on your security requirements, consider:
- Validating the invitation code format (e.g., length, allowed characters)
- Adding a configuration option to enable/disable invitation code handling
- Adding rate limiting to prevent abuse
Example validation:
if (hasInvitationCode) { try { const invitationCode = params.get("invitation_code"); // Validate format (example: alphanumeric, 8-32 chars) if (!invitationCode || !/^[a-zA-Z0-9]{8,32}$/.test(invitationCode)) { if (config.isDebugMode) { console.warn("authMiddleware: invalid invitation code format"); } return NextResponse.next(); // or redirect to error page } // ... rest of the logic
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
src/authMiddleware/authMiddleware.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-21T09:58:35.193Z
Learnt from: DanielRivers
Repo: kinde-oss/kinde-auth-nextjs PR: 243
File: src/authMiddleware/authMiddleware.ts:46-49
Timestamp: 2024-11-21T09:58:35.193Z
Learning: In `src/authMiddleware/authMiddleware.ts`, the `validateToken` function from `kinde/jwt-validator` handles exceptions internally, so additional error handling is not necessary when calling it.
Applied to files:
src/authMiddleware/authMiddleware.ts
📚 Learning: 2025-01-31T17:11:40.324Z
Learnt from: DanielRivers
Repo: kinde-oss/kinde-auth-nextjs PR: 264
File: src/frontend/AuthProvider.jsx:54-54
Timestamp: 2025-01-31T17:11:40.324Z
Learning: In the Kinde Auth NextJS SDK, route validation and default fallbacks for undefined routes are handled in the config/index.ts file through the validateRoute function, making additional route validation unnecessary in components.
Applied to files:
src/authMiddleware/authMiddleware.ts
📚 Learning: 2024-12-18T12:34:59.343Z
Learnt from: Yoshify
Repo: kinde-oss/kinde-auth-nextjs PR: 254
File: src/session/isAuthenticated.js:15-19
Timestamp: 2024-12-18T12:34:59.343Z
Learning: In Next.js, calling the built-in redirect() function internally throws an error. If we wrap that call in a try/catch, we inadvertently catch the error and block the intended redirection flow.
Applied to files:
src/authMiddleware/authMiddleware.ts
📚 Learning: 2024-12-17T00:41:07.608Z
Learnt from: Yoshify
Repo: kinde-oss/kinde-auth-nextjs PR: 254
File: src/session/isAuthenticated.js:14-17
Timestamp: 2024-12-17T00:41:07.608Z
Learning: In `src/session/isAuthenticated.js` of this Next.js application, cookies cannot be modified in React Server Components (RSC). Therefore, to prevent accessing stale data outside of middleware, the application redirects on token expiry.
Applied to files:
src/authMiddleware/authMiddleware.ts
🧬 Code graph analysis (1)
src/authMiddleware/authMiddleware.ts (2)
src/handlers/protect.js (1)
params(22-22)src/config/index.ts (1)
config(107-144)
🔇 Additional comments (1)
src/authMiddleware/authMiddleware.ts (1)
37-65: Verify the placement of invitation code handling in the middleware flow.The invitation code logic executes before public path checks and authentication checks, which means authenticated users visiting any page with an
invitation_codeparameter will be redirected to the registration page, potentially disrupting their session. Confirm whether this behavior aligns with the intended invitation flow — particularly whether the register page is designed to handle already-authenticated users gracefully, and whether the invitation code should instead flow through the OAuth authorization flow per Kinde's recommended pattern.
| const params = new URLSearchParams(search); | ||
| const hasInvitationCode = params.has("invitation_code"); |
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 that the invitation code has a non-empty value.
params.has("invitation_code") returns true even when the query parameter is present but empty (e.g., ?invitation_code=). This could lead to redirecting with an empty invitation code.
Consider checking for a non-empty value:
const params = new URLSearchParams(search);
- const hasInvitationCode = params.has("invitation_code");
+ const invitationCode = params.get("invitation_code");
+ const hasInvitationCode = invitationCode && invitationCode.trim().length > 0;Then use the already-extracted invitationCode variable at line 40 instead of calling params.get() again.
📝 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.
| const params = new URLSearchParams(search); | |
| const hasInvitationCode = params.has("invitation_code"); | |
| const params = new URLSearchParams(search); | |
| const invitationCode = params.get("invitation_code"); | |
| const hasInvitationCode = invitationCode && invitationCode.trim().length > 0; |
🤖 Prompt for AI Agents
In src/authMiddleware/authMiddleware.ts around lines 20-21, replace the boolean
check that uses params.has("invitation_code") with a check that retrieves the
value once (const invitationCode = params.get("invitation_code")), trims it, and
verifies it is non-empty (e.g., invitationCode != null &&
invitationCode.trim().length > 0); set hasInvitationCode based on that truthy
non-empty test and later reuse the already-extracted invitationCode at line 40
instead of calling params.get() again.
| const invitationCode = params.get("invitation_code"); | ||
| registerWithInviteRedirectUrlParams.set( | ||
| "invitation_code", | ||
| invitationCode, | ||
| ); |
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.
Handle potential null value from invitation_code parameter.
params.get("invitation_code") can return null if the parameter is not present (though hasInvitationCode guards against this) or an empty string if the parameter is present without a value. The code doesn't handle these cases before using the value.
If you implement the validation suggested in the previous comment (lines 20-21), this will be addressed. Otherwise, add a null check here:
const invitationCode = params.get("invitation_code");
+ if (!invitationCode) {
+ throw new Error("Invalid invitation code");
+ }
registerWithInviteRedirectUrlParams.set(
"invitation_code",
invitationCode,
);📝 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.
| const invitationCode = params.get("invitation_code"); | |
| registerWithInviteRedirectUrlParams.set( | |
| "invitation_code", | |
| invitationCode, | |
| ); | |
| const invitationCode = params.get("invitation_code"); | |
| if (!invitationCode) { | |
| throw new Error("Invalid invitation code"); | |
| } | |
| registerWithInviteRedirectUrlParams.set( | |
| "invitation_code", | |
| invitationCode, | |
| ); |
🤖 Prompt for AI Agents
In src/authMiddleware/authMiddleware.ts around lines 40 to 44,
params.get("invitation_code") may return null or an empty string; update the
code to validate before using it by checking that invitationCode is not null and
not an empty string (or coerce to a safe default) and only call
registerWithInviteRedirectUrlParams.set("invitation_code", invitationCode) when
the value is valid; alternatively, ensure the earlier hasInvitationCode guard
guarantees a non-null/non-empty value so this assignment is safe.
|
Good Morning, @pesickaa. I tested the invitation code flow and it works well. I think the code rabbit suggestions are valid as when I tested I did see I could produce some odd behavior. I do want to verify something with you, when an authenticated user hits a page with ?invitation_code=X, they get redirected to register. Is that intentional? I tested with /dashboard?invitation_code=123 and it redirected even when logged in. Otherwise looks good to merge, great work. |
|
@KeeganBeuthin Yes, this is a valid use case. Consider this: A SASS product with multiple businesses, the user could be logged into one and invited into another, they shouldn't be blocked to joining the 2nd business |
|
@DanielRivers That is a fair use case, thank you for clarifying, happy with that. Once the rabbit changes are made I think it's good to go |
Explain your changes
Add support for invitation codes so users are prompted to login or register
Testing
To test this, pull down this branch.
Suppose there is a related issue with enough detail for a reviewer to understand your changes fully. In that case, you can omit an explanation and instead include either “Fixes #XX” or “Updates #XX” where “XX” is the issue number.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.