Implemented invitation code functionality, removed unnecessary md files#216
Implemented invitation code functionality, removed unnecessary md files#216Koosha-Owji merged 7 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe pull request adds invitation code support across the Kinde authentication library, enabling authenticated flows via invitation links. Changes include interface methods, session implementations, request parameter handling, servlet/filter integration, Spring Boot OAuth2 resolver customization, and comprehensive test coverage across all layers. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User<br/>(Request)
participant Filter as Filter/<br/>Servlet
participant Session as KindeClientSession
participant Auth as AuthorizationUrl
participant OAuth as OAuth2 Server
User->>Filter: Request with invitation_code
Filter->>Filter: Extract & validate invitation_code
alt Valid invitation_code present
Filter->>Session: login(invitationCode)
Session->>Session: addInvitationParams()
Session->>Auth: Build AuthorizationUrl<br/>(+invitation_code, +is_invitation)
Auth->>OAuth: Redirect with params
else No/Invalid invitation_code
Filter->>Filter: Fall through to standard flow
Filter->>Session: login() / register()
Session->>Auth: Build standard AuthorizationUrl
Auth->>OAuth: Redirect
end
OAuth-->>User: Authorization redirect
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. ✨ 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
playground/kinde-j2ee-app/pom.xml (1)
49-53: Consider addingruntimescope to slf4j-simple.The dependency has no scope specified, defaulting to
compile. Since SLF4J bindings are only needed at runtime (not at compile time), usingruntimescope is more appropriate and consistent with the pattern used in similar projects likekinde-springboot-starter-example(which usesruntimescope forlogback-classic).Suggested change
<dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-simple</artifactId> <version>2.0.17</version> + <scope>runtime</scope> </dependency>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/kinde-j2ee-app/pom.xml` around lines 49 - 53, The slf4j binding dependency (groupId org.slf4j, artifactId slf4j-simple) is declared with default compile scope; change its Maven scope to runtime so the SLF4J binding is available only at runtime (not at compile time) by adding <scope>runtime</scope> to the slf4j-simple dependency entry in the POM.kinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolver.java (1)
23-24: Reuse the shared invitation parameter constants instead of re-inlining them.
invitation_codeandis_invitationalready exist in core, and this class is now another place that has to stay perfectly in sync with them.♻️ Suggested cleanup
+import com.kinde.session.KindeRequestParameters; import jakarta.servlet.http.HttpServletRequest; @@ - private static final String INVITATION_CODE_PARAM = "invitation_code"; - private static final String IS_INVITATION_PARAM = "is_invitation"; + private static final String INVITATION_CODE_PARAM = KindeRequestParameters.INVITATION_CODE; + private static final String IS_INVITATION_PARAM = KindeRequestParameters.IS_INVITATION; @@ - additionalParams.put(IS_INVITATION_PARAM, "true"); + additionalParams.put(IS_INVITATION_PARAM, Boolean.TRUE.toString());As per coding guidelines: Please focus on API design and consistency.
Also applies to: 53-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolver.java` around lines 23 - 24, This class defines local constants INVITATION_CODE_PARAM and IS_INVITATION_PARAM that duplicate core/shared invitation parameter names; remove these local constants and switch all usages in KindeOAuth2AuthorizationRequestResolver (including the spots around the current usages ~lines 53-59) to reference the single source-of-truth constants from the core/shared class (import that class and use its constant names instead), ensuring no duplicate literals remain and API names stay consistent across modules.kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java (1)
34-85: Add at least one test forresolve(HttpServletRequest, String clientRegistrationId).The class exposes two public resolution paths, but this suite only exercises the no-
clientRegistrationIdoverload. A small happy-path test for the second overload would protect the wiring you added in Lines 43-45.🧪 Suggested test
+ `@Test` + `@DisplayName`("Resolve with invitation_code and registration id adds invitation params") + void resolveWithInvitationCodeAddsParamsForClientRegistrationId() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/kinde"); + request.setServletPath("/oauth2/authorization/kinde"); + request.setParameter("invitation_code", "inv_abc123"); + + OAuth2AuthorizationRequest authRequest = resolver.resolve(request, "kinde"); + + assertNotNull(authRequest); + assertEquals("inv_abc123", authRequest.getAdditionalParameters().get("invitation_code")); + assertEquals("true", authRequest.getAdditionalParameters().get("is_invitation")); + }As per coding guidelines: Validate test coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java` around lines 34 - 85, Add a unit test that covers the overloaded method resolve(HttpServletRequest, String) by creating a MockHttpServletRequest with servletPath "/oauth2/authorization/kinde", setting invitation_code to "inv_abc123", and calling resolver.resolve(request, "kinde"); assert the returned OAuth2AuthorizationRequest is not null and that getAdditionalParameters() contains "invitation_code" == "inv_abc123" and "is_invitation" == "true" (mirror the existing no-arg test logic), and also add a complementary assertion that resolve(request, "otherId") returns null or does not include the invitation params to validate clientRegistrationId wiring.kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java (1)
155-213: AddCREATE_ORGinvitation coverage to this suite.The new tests cover login/register, but the filter now has a separate invitation-aware
CREATE_ORGpath as well. A positive case and a missing-org_namecase would catch regressions in both the branch selection and the validation message.As per coding guidelines,
**/*: focus on test coverage and quality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 155 - 213, Add two tests exercising the CREATE_ORG branch: (1) a positive test that sets request.getParameter("invitation_code") and request.getParameter("org_name"), stubs an AuthorizationUrl, stubs when(mockSession.createOrganization("org_name_value","inv_code")).thenReturn(invitationAuthUrl), calls filter.doFilter(..., KindeAuthenticationAction.CREATE_ORG) and verifies mockSession.createOrganization(...) was called, session.setAttribute(AUTHORIZATION_URL, invitationAuthUrl) and response.sendRedirect(invitationAuthUrl.getUrl().toString()) and that filterChain.doFilter was never invoked; (2) a validation test that sets invitation_code but leaves org_name empty (or null), calls filter.doFilter(..., KindeAuthenticationAction.CREATE_ORG) and verifies mockSession.createOrganization(...) was not called and the request/flow is left for validation (e.g., filterChain.doFilter invoked or the expected validation handler/attribute is set) to assert the missing-org_name case is handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kinde-core/src/main/java/com/kinde/KindeClientSession.java`:
- Around line 20-30: The new abstract methods will break implementors; change
the invitation-aware overloads (createOrg(String), register(String),
login(String invitationCode)) into default methods on KindeClientSession that
delegate to the existing no-arg/legacy variants (e.g., default createOrg(String
orgName, String invitationCode) should call createOrg(orgName), default
register(String invitationCode) should call register(), and default login(String
invitationCode) should call login()); also make handleInvitation(String
invitationCode) a default method that throws UnsupportedOperationException (or
returns an appropriate unsupported result) so existing implementations remain
source-compatible unless they opt in to support invitations.
In `@kinde-core/src/main/java/com/kinde/session/KindeClientCodeSessionImpl.java`:
- Around line 84-91: The TokenResponse is being parsed directly from
httpRequest.send(), which discards the raw HTTP response body needed for
debugging non-standard errors; modify retrieveTokens so you first capture the
raw HTTP response (e.g., response body string from the HttpResponse returned by
httpRequest.send()), then parse that body into TokenResponse/TokenErrorResponse
(TokenResponse.parse(...)) and, when throwing the Exception in the error branch,
include the raw response body alongside the existing errorDetail so the thrown
exception contains the original token endpoint response for troubleshooting.
In `@kinde-core/src/main/java/com/kinde/session/KindeClientSessionImpl.java`:
- Around line 300-304: In addInvitationParams (class KindeClientSessionImpl)
change the blank-check to reject whitespace-only invitation codes by using
String.isBlank() instead of isEmpty() so calls like login(" ") won't add
KindeRequestParameters.INVITATION_CODE or set
KindeRequestParameters.IS_INVITATION; update the conditional that currently
reads invitationCode != null && !invitationCode.isEmpty() to use invitationCode
!= null && !invitationCode.isBlank() to ensure all invitation-aware entry points
behave consistently.
In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java`:
- Around line 54-60: The invitation handling currently treats whitespace-only
values as present because it uses rawInvitationCode != null &&
!rawInvitationCode.isEmpty(); update the check in KindeAuthenticationFilter so
whitespace-only strings are treated as absent (e.g., use rawInvitationCode !=
null && !rawInvitationCode.isBlank() or trim() equivalently) when assigning
invitationCode, so that the invitation flow (createKindeClientSession and
subsequent logic) is only triggered for non-blank codes; adjust the assignment
of invitationCode and any related null checks around createKindeClientSession
and AUTHORIZATION_URL handling to match this change.
In
`@kinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/Kinde.java`:
- Around line 29-33: The reactive overload
configureOAuth2WithPkce(ServerHttpSecurity, ...) currently uses
DefaultServerOAuth2AuthorizationRequestResolver and drops invitation parameters,
causing inconsistent behavior with the servlet overload; replace the default
resolver with a reactive equivalent that forwards invitation params (e.g.
implement or reuse a KindeServerOAuth2AuthorizationRequestResolver and
instantiate it where configureOAuth2WithPkce(ServerHttpSecurity, ...) sets up
the authorizationRequestResolver) so invitation handling matches the servlet
path (or explicitly document the servlet-only limitation if you choose not to
implement the reactive resolver).
In `@playground/kinde-j2ee-app/pom.xml`:
- Around line 75-87: The pom currently specifies jetty-ee10-maven-plugin with
<artifactId>jetty-ee10-maven-plugin</artifactId> and <version>12.0.16</version>;
update that version to 12.0.17 or later to address CVE-2025-1948 by editing the
plugin entry (the <version> value) so builds use 12.0.17+, then run a quick
build to verify the plugin loads correctly.
---
Nitpick comments:
In
`@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java`:
- Around line 155-213: Add two tests exercising the CREATE_ORG branch: (1) a
positive test that sets request.getParameter("invitation_code") and
request.getParameter("org_name"), stubs an AuthorizationUrl, stubs
when(mockSession.createOrganization("org_name_value","inv_code")).thenReturn(invitationAuthUrl),
calls filter.doFilter(..., KindeAuthenticationAction.CREATE_ORG) and verifies
mockSession.createOrganization(...) was called,
session.setAttribute(AUTHORIZATION_URL, invitationAuthUrl) and
response.sendRedirect(invitationAuthUrl.getUrl().toString()) and that
filterChain.doFilter was never invoked; (2) a validation test that sets
invitation_code but leaves org_name empty (or null), calls filter.doFilter(...,
KindeAuthenticationAction.CREATE_ORG) and verifies
mockSession.createOrganization(...) was not called and the request/flow is left
for validation (e.g., filterChain.doFilter invoked or the expected validation
handler/attribute is set) to assert the missing-org_name case is handled.
In
`@kinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolver.java`:
- Around line 23-24: This class defines local constants INVITATION_CODE_PARAM
and IS_INVITATION_PARAM that duplicate core/shared invitation parameter names;
remove these local constants and switch all usages in
KindeOAuth2AuthorizationRequestResolver (including the spots around the current
usages ~lines 53-59) to reference the single source-of-truth constants from the
core/shared class (import that class and use its constant names instead),
ensuring no duplicate literals remain and API names stay consistent across
modules.
In
`@kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java`:
- Around line 34-85: Add a unit test that covers the overloaded method
resolve(HttpServletRequest, String) by creating a MockHttpServletRequest with
servletPath "/oauth2/authorization/kinde", setting invitation_code to
"inv_abc123", and calling resolver.resolve(request, "kinde"); assert the
returned OAuth2AuthorizationRequest is not null and that
getAdditionalParameters() contains "invitation_code" == "inv_abc123" and
"is_invitation" == "true" (mirror the existing no-arg test logic), and also add
a complementary assertion that resolve(request, "otherId") returns null or does
not include the invitation params to validate clientRegistrationId wiring.
In `@playground/kinde-j2ee-app/pom.xml`:
- Around line 49-53: The slf4j binding dependency (groupId org.slf4j, artifactId
slf4j-simple) is declared with default compile scope; change its Maven scope to
runtime so the SLF4J binding is available only at runtime (not at compile time)
by adding <scope>runtime</scope> to the slf4j-simple dependency entry in the
POM.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2dbd236e-9fe6-4272-94bf-0edfb2b97f6c
⛔ Files ignored due to path filters (11)
ENTITLEMENTS_IMPLEMENTATION.mdis excluded by!**/*.mdand included by noneENTITLEMENTS_IMPLEMENTATION_SUMMARY.mdis excluded by!**/*.mdand included by noneHARD_CHECK_SUMMARY.mdis excluded by!**/*.mdand included by noneTEST_COVERAGE.mdis excluded by!**/*.mdand included by noneTEST_SUMMARY.mdis excluded by!**/*.mdand included by noneVERIFICATION.mdis excluded by!**/*.mdand included by nonekinde-core/AUTH_WRAPPERS_README.mdis excluded by!**/*.mdand included by nonekinde-core/ENTITLEMENTS_README.mdis excluded by!**/*.mdand included by nonekinde-core/HARD_CHECK_IMPLEMENTATION.mdis excluded by!**/*.mdand included by noneplayground/kinde-j2ee-app/src/main/webapp/index.jspis excluded by none and included by noneplayground/kinde-springboot-starter-example/src/main/resources/templates/index.htmlis excluded by none and included by none
📒 Files selected for processing (17)
kinde-core/src/main/java/com/kinde/KindeClientSession.javakinde-core/src/main/java/com/kinde/constants/KindeConstants.javakinde-core/src/main/java/com/kinde/session/KindeClientCodeSessionImpl.javakinde-core/src/main/java/com/kinde/session/KindeClientSessionImpl.javakinde-core/src/main/java/com/kinde/session/KindeRequestParameters.javakinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.javakinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.javakinde-j2ee/src/main/java/com/kinde/servlet/KindeAuthenticationServlet.javakinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.javakinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.javakinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/Kinde.javakinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolver.javakinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.javaplayground/kinde-core-example/src/test/java/com/kinde/KindeCoreExampleTest.javaplayground/kinde-j2ee-app/pom.xmlplayground/kinde-springboot-starter-example/src/main/java/com/kinde/spring/config/SecurityConfig.javaplayground/kinde-springboot-starter-example/src/main/java/com/kinde/spring/resolver/CustomAuthorizationRequestResolver.java
kinde-core/src/main/java/com/kinde/session/KindeClientCodeSessionImpl.java
Outdated
Show resolved
Hide resolved
kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java
Show resolved
Hide resolved
kinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/Kinde.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java (2)
185-200: Missing assertion for session attribute update.This test verifies that an existing session is overridden when an invitation code is present, but unlike
testInvitationCodeOnLoginPassesCodeToLogin, it doesn't verify thatAUTHORIZATION_URLis updated in the session. This inconsistency could mask a bug.💡 Suggested fix
verify(mockSession).login("inv_override"); + verify(session).setAttribute(AUTHORIZATION_URL, invitationAuthUrl); verify(response).sendRedirect(invitationAuthUrl.getUrl().toString()); verify(filterChain, never()).doFilter(any(ServletRequest.class), any(ServletResponse.class));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 185 - 200, Add an assertion to testInvitationCodeOverridesExistingSession to verify that the session's AUTHORIZATION_URL attribute is updated with the new invitationAuthUrl after login; specifically, after calling filter.doFilter(...), verify that session.setAttribute(AUTHORIZATION_URL, invitationAuthUrl) (or mockSession.setAttribute(...) if login uses mockSession) is invoked so the test mirrors testInvitationCodeOnLoginPassesCodeToLogin's session-update check.
202-213: Incomplete assertions for fallthrough behavior.This test verifies that an empty invitation code falls back to
login()without arguments, but unliketestRedirectToAuthorizationWhenNoPrincipalOrAuthUrl, it doesn't verify the redirect or filter chain behavior. Consider adding assertions for consistency.💡 Suggested fix
filter.doFilter(request, response, filterChain, KindeAuthenticationAction.LOGIN); verify(mockSession).login(); + verify(session).setAttribute(AUTHORIZATION_URL, mockAuthUrl); + verify(response).sendRedirect("http://auth.url"); + verify(filterChain, never()).doFilter(any(ServletRequest.class), any(ServletResponse.class)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 202 - 213, The test testEmptyInvitationCodeFallsThrough currently only verifies mockSession.login() was called; extend it to assert the downstream behavior matches the other test by verifying response.sendRedirect(...) was invoked with the URL from mockAuthUrl.getUrl() and that filterChain.doFilter(request, response) was NOT invoked (or vice-versa if the intended behavior is to continue the chain); update the assertions around filter.doFilter(request, response, filterChain, KindeAuthenticationAction.LOGIN) to verify response.sendRedirect(mockAuthUrl.getUrl().toString()) and verify(filterChain, never()).doFilter(request, response) so the fall-through behavior is explicitly tested alongside mockSession.login().kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.java (1)
147-150: Assert decoded query params instead of raw substrings.These checks only use
url.contains(...)with URL-safe literals, so they won't catch regressions around encoding, duplicate keys, or wrong values hidden inside a longer query string. Parsing the query once and asserting exact parameter values would make these invitation tests much more reliable, especially if you add one case with reserved characters ininvitationCodeororg_name.
As per coding guidelines, "Please focus on: ... Test coverage and quality".Also applies to: 160-163, 174-176, 187-190, 201-202, 213-214, 225-226, 237-239, 250-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.java` around lines 147 - 150, The tests currently assert raw URL substrings (using result.getUrl().toString().contains(...)) which can miss encoding/duplication issues; update each assertion block (e.g., in KindeClientSessionImplTest where result.getUrl() is used) to parse the URL's query string once (use URI/URL or a query parser), decode parameters, and assert exact parameter values (e.g., invitation_code == "inv_login123", is_invitation == "true", supports_reauth == "true"); apply the same change to the other similar blocks referenced (lines around the repeated occurrences) so each test verifies decoded parameter names and values rather than raw substrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java`:
- Around line 34-45: Update the test resolveWithInvitationCodeAddsParams so it
also asserts that the emitted redirect URI includes the invitation parameters:
after obtaining OAuth2AuthorizationRequest authRequest from
resolver.resolve(request), add assertions against
authRequest.getAuthorizationRequestUri() to verify it contains both
"invitation_code=inv_abc123" and "is_invitation=true" (URL-encoded if necessary)
in the query string; keep existing additionalParameters assertions on
authRequest.getAdditionalParameters() and ensure you URL-check the full URI
returned by getAuthorizationRequestUri() rather than only the parameter map.
- Around line 101-112: Add a counterpart test that verifies the two-argument
overload of OAuth2AuthorizationRequestResolver does not add empty invitation
params: create a MockHttpServletRequest like in
resolveWithClientRegistrationIdWithoutInvitationCodeDoesNotAddParams but include
setParameter("invitation_code", "") and/or setParameter("is_invitation", "") on
the request, call resolver.resolve(request, "kinde"), then assert authRequest is
not null and that authRequest.getAdditionalParameters().get("invitation_code")
and get("is_invitation") are null; name the new test e.g.
resolveWithClientRegistrationIdWithEmptyInvitationCodeDoesNotAddParams to mirror
the existing one and locate it near the other tests in
KindeOAuth2AuthorizationRequestResolverTest.
---
Nitpick comments:
In `@kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.java`:
- Around line 147-150: The tests currently assert raw URL substrings (using
result.getUrl().toString().contains(...)) which can miss encoding/duplication
issues; update each assertion block (e.g., in KindeClientSessionImplTest where
result.getUrl() is used) to parse the URL's query string once (use URI/URL or a
query parser), decode parameters, and assert exact parameter values (e.g.,
invitation_code == "inv_login123", is_invitation == "true", supports_reauth ==
"true"); apply the same change to the other similar blocks referenced (lines
around the repeated occurrences) so each test verifies decoded parameter names
and values rather than raw substrings.
In
`@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java`:
- Around line 185-200: Add an assertion to
testInvitationCodeOverridesExistingSession to verify that the session's
AUTHORIZATION_URL attribute is updated with the new invitationAuthUrl after
login; specifically, after calling filter.doFilter(...), verify that
session.setAttribute(AUTHORIZATION_URL, invitationAuthUrl) (or
mockSession.setAttribute(...) if login uses mockSession) is invoked so the test
mirrors testInvitationCodeOnLoginPassesCodeToLogin's session-update check.
- Around line 202-213: The test testEmptyInvitationCodeFallsThrough currently
only verifies mockSession.login() was called; extend it to assert the downstream
behavior matches the other test by verifying response.sendRedirect(...) was
invoked with the URL from mockAuthUrl.getUrl() and that
filterChain.doFilter(request, response) was NOT invoked (or vice-versa if the
intended behavior is to continue the chain); update the assertions around
filter.doFilter(request, response, filterChain, KindeAuthenticationAction.LOGIN)
to verify response.sendRedirect(mockAuthUrl.getUrl().toString()) and
verify(filterChain, never()).doFilter(request, response) so the fall-through
behavior is explicitly tested alongside mockSession.login().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d3289ab-201a-4d4e-ae63-1ec9cd0542b0
📒 Files selected for processing (4)
kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.javakinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.javakinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.javakinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java
...ingboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java
Show resolved
Hide resolved
...ingboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java
Show resolved
Hide resolved
…or invitation code feature
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java (1)
137-138:⚠️ Potential issue | 🟡 MinorUse URL-safe Base64 encoder to match production decoder.
The tests use
java.util.Base64.getEncoder()(standard Base64), but the production code usesBase64.getUrlDecoder()(URL-safe Base64). While these may work interchangeably for simple ASCII test strings, they differ for inputs containing+,/, or=characters. Using the matching encoder ensures tests accurately reflect production behavior and won't pass incorrectly if the test data changes.Proposed fix
- String encodedState = java.util.Base64.getEncoder().encodeToString(urlParams.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + String encodedState = java.util.Base64.getUrlEncoder().encodeToString(urlParams.getBytes(java.nio.charset.StandardCharsets.UTF_8));Apply this change to both
testLoginLinkExpiredRedirectsWithDecodedParams(line 138) andtestLoginLinkExpiredWithQueryParamsInAuthUrl(line 255).Also applies to: 254-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 137 - 138, Tests currently encode state with java.util.Base64.getEncoder() while production uses Base64.getUrlDecoder(); update the tests to use the URL-safe encoder by replacing getEncoder() with java.util.Base64.getUrlEncoder() when building encodedState in the test methods testLoginLinkExpiredRedirectsWithDecodedParams and testLoginLinkExpiredWithQueryParamsInAuthUrl so the encoded state matches the URL-safe decoder used in production.playground/kinde-management-example/pom.xml (1)
63-67:⚠️ Potential issue | 🟡 MinorRemove or explicitly set the empty SLF4J binding scope.
An empty
<scope></scope>element is invalid in Maven—after content trimming, it becomes an empty string, which is not a valid scope value and produces build validation warnings. Either explicitly set it to<scope>runtime</scope>if the logger backend should be available only at runtime, or omit the element entirely to use Maven's defaultcompilescope.♻️ Proposed fix
<dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-simple</artifactId> <version>2.0.17</version> - <scope></scope> + <scope>runtime</scope> </dependency>Alternatively, remove the
<scope>tag entirely ifcompilescope is acceptable:<dependency> <groupId>org.slf4j</groupId> <artifactId>slf4j-simple</artifactId> <version>2.0.17</version> - <scope></scope> </dependency>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/kinde-management-example/pom.xml` around lines 63 - 67, The SLF4J dependency block for org.slf4j:slf4j-simple contains an empty <scope></scope>, which is invalid; remove the empty <scope> element or replace it with an explicit valid scope (e.g., set <scope>runtime</scope>) in the dependency declaration for org.slf4j:slf4j-simple so Maven no longer emits build validation warnings.
♻️ Duplicate comments (3)
kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java (2)
115-126:⚠️ Potential issue | 🟡 MinorMirror the blank-value coverage for the
clientRegistrationIdoverload.
resolveWithClientRegistrationIdWithoutInvitationCodeDoesNotAddParams()does not cover the empty or whitespace-only cases that the single-arg overload already exercises. That leavesresolver.resolve(request, "kinde")free to start forwarding blank invitation values unnoticed.Companion test
+ `@Test` + `@DisplayName`("Resolve with clientRegistrationId and empty invitation_code does not add params") + void resolveWithClientRegistrationIdAndEmptyInvitationCodeDoesNotAddParams() { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/kinde"); + request.setServletPath("/oauth2/authorization/kinde"); + request.setParameter("invitation_code", ""); + + OAuth2AuthorizationRequest authRequest = resolver.resolve(request, "kinde"); + + assertNotNull(authRequest); + assertNull(authRequest.getAdditionalParameters().get("invitation_code")); + assertNull(authRequest.getAdditionalParameters().get("is_invitation")); + }As per coding guidelines, focus on "Test coverage and quality" and "Validate test coverage".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java` around lines 115 - 126, The test resolveWithClientRegistrationIdWithoutInvitationCodeDoesNotAddParams currently checks the absent-invitation case but misses blank/whitespace invitation values; update this test (or add companion tests) to also set request parameters "invitation_code" to "" and to a whitespace-only string (e.g., " "), call resolver.resolve(request, "kinde") for each case, and assert authRequest is not null and that authRequest.getAdditionalParameters().get("invitation_code") and get("is_invitation") are null; reference the resolver.resolve(request, "kinde") call and the assertions on authRequest.getAdditionalParameters() to locate where to add these checks.
34-46:⚠️ Potential issue | 🟠 MajorAssert the emitted authorization URI too.
These cases still only verify
additionalParameters. A resolver that forgets to propagateinvitation_codeandis_invitationinto the final redirect URI would still pass this suite while dropping them on the wire.Suggested test hardening
assertNotNull(authRequest); assertEquals("inv_abc123", authRequest.getAdditionalParameters().get("invitation_code")); assertEquals("true", authRequest.getAdditionalParameters().get("is_invitation")); + assertTrue(authRequest.getAuthorizationRequestUri().contains("invitation_code=inv_abc123")); + assertTrue(authRequest.getAuthorizationRequestUri().contains("is_invitation=true"));assertNotNull(authRequest); assertEquals("inv_client_reg", authRequest.getAdditionalParameters().get("invitation_code")); assertEquals("true", authRequest.getAdditionalParameters().get("is_invitation")); + assertTrue(authRequest.getAuthorizationRequestUri().contains("invitation_code=inv_client_reg")); + assertTrue(authRequest.getAuthorizationRequestUri().contains("is_invitation=true"));As per coding guidelines, focus on "Test coverage and quality" and "API design and consistency".
Also applies to: 101-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java` around lines 34 - 46, The test resolveWithInvitationCodeAddsParams currently only checks OAuth2AuthorizationRequest.getAdditionalParameters; update it to also assert that the emitted authorization URI (from OAuth2AuthorizationRequest.getAuthorizationRequestUri()) contains the invitation parameters so they are sent on the wire: after obtaining authRequest from resolver.resolve(request), assert authRequest.getAuthorizationRequestUri() is non-null and contains both "invitation_code=inv_abc123" and "is_invitation=true" (URL-encoded as needed) to ensure resolver.resolve(...) actually propagates these into the redirect URI.kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java (1)
54-55:⚠️ Potential issue | 🟡 MinorTrim nonblank invitation codes before using them.
Whitespace-only input is handled now, but values like
" inv123 "still get forwarded with the spaces intact. That turns a recoverable input issue into an invalid auth request.Suggested fix
- String invitationCode = (rawInvitationCode != null && !rawInvitationCode.isBlank()) ? rawInvitationCode : null; + String invitationCode = (rawInvitationCode != null && !rawInvitationCode.isBlank()) + ? rawInvitationCode.trim() + : null;As per coding guidelines,
**/*: focus on error handling and edge cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java` around lines 54 - 55, In KindeAuthenticationFilter, rawInvitationCode is currently checked for non-blank but not trimmed, so values like " inv123 " are forwarded; replace the logic that sets invitationCode to first trim rawInvitationCode (e.g., String trimmed = rawInvitationCode != null ? rawInvitationCode.trim() : null) then set invitationCode to trimmed if trimmed is not blank, otherwise null, using the existing INVITATION_CODE parameter handling to ensure leading/trailing whitespace is removed before use.
🧹 Nitpick comments (3)
kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java (2)
202-226: Tests could verify full fallthrough behavior.These tests only verify that
login()(no-arg) is called when invitation_code is empty or whitespace. Consider adding assertions to verify the redirect happens as expected (similar totestRedirectToAuthorizationWhenNoPrincipalOrAuthUrl), which would ensure the complete fallthrough behavior is tested.Example enhancement for testEmptyInvitationCodeFallsThrough
filter.doFilter(request, response, filterChain, KindeAuthenticationAction.LOGIN); verify(mockSession).login(); + verify(response).sendRedirect("http://auth.url"); + verify(filterChain, never()).doFilter(any(ServletRequest.class), any(ServletResponse.class)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 202 - 226, The tests testEmptyInvitationCodeFallsThrough and testWhitespaceOnlyInvitationCodeFallsThrough currently only verify mockSession.login() is invoked; extend each to assert the full fallthrough behavior by also verifying the redirect occurs (like in testRedirectToAuthorizationWhenNoPrincipalOrAuthUrl): assert response.sendRedirect(...) is called with mockAuthUrl.getUrl().toString() (or verify filterChain.doFilter(...) is not invoked) after filter.doFilter(...), and ensure you keep the existing when(mockSession.login()).thenReturn(mockAuthUrl) setup so the expected redirect URL is available.
185-200: Consider adding session attribute verification for completeness.The test verifies the redirect and method call, but unlike
testInvitationCodeOnLoginPassesCodeToLogin, it doesn't verify thatsession.setAttribute(AUTHORIZATION_URL, invitationAuthUrl)is called. Adding this assertion would ensure the session state is correctly updated when overriding an existing session.Proposed fix
filter.doFilter(request, response, filterChain, KindeAuthenticationAction.LOGIN); verify(mockSession).login("inv_override"); + verify(session).setAttribute(AUTHORIZATION_URL, invitationAuthUrl); verify(response).sendRedirect(invitationAuthUrl.getUrl().toString()); verify(filterChain, never()).doFilter(any(ServletRequest.class), any(ServletResponse.class)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 185 - 200, The test testInvitationCodeOverridesExistingSession should also assert that the session's AUTHORIZATION_URL is updated; add a verification that session.setAttribute(AUTHORIZATION_URL, invitationAuthUrl) is called after filter.doFilter(...) so the session state is validated (locate in KindeAuthenticationFilterTest near the existing verify(mockSession).login("inv_override") and verify(response).sendRedirect(...) assertions).kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java (1)
152-164: AssertPOST_LOGIN_URLis preserved in the register invitation flow.This test passes
POST_LOGIN_URL, but unlike the login/create-org variants it never verifies that the servlet stores it in session. That leaves the post-registration redirect contract untested.Suggested assertion
verify(mockSession).register("inv_reg456"); verify(session).setAttribute(AUTHORIZATION_URL, mockAuthUrl); + verify(session).setAttribute(POST_LOGIN_URL, "http://example.com/dashboard"); verify(response).sendRedirect(mockAuthUrl.getUrl().toString());As per coding guidelines,
**/*: focus on test coverage and quality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java` around lines 152 - 164, The test doesn't assert that POST_LOGIN_URL is saved to the session during the register-with-invitation flow; after calling servlet.doGet(request, response, KindeAuthenticationAction.REGISTER) add a verification that the session setAttribute was called with POST_LOGIN_URL and the passed URL (e.g. verify(session).setAttribute(POST_LOGIN_URL, "http://example.com/dashboard")); place this alongside the existing verify(session).setAttribute(AUTHORIZATION_URL, mockAuthUrl) assertion so the post-registration redirect contract is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kinde-core/src/main/java/com/kinde/session/KindeClientSessionImpl.java`:
- Around line 293-298: handleInvitation currently accepts null/blank
invitationCode and proceeds to build an authorization URL; instead validate the
input up-front in handleInvitation (e.g., check invitationCode for null or
blank) and throw an appropriate runtime exception such as
IllegalArgumentException when the caller provides invalid input so bugs aren't
masked; keep using addInvitationParams and authorizationUrlWithParameters after
the validation so normal flow is unchanged for valid codes.
In `@kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.java`:
- Around line 229-239: The test loginWithWhitespaceOnlyCodeOmitsInvitationParams
currently only checks that invitation params are omitted; add an assertion that
the login-specific flag is still present by verifying the generated
AuthorizationUrl includes supports_reauth=true (e.g.,
assertTrue(url.contains("supports_reauth=true"))), ensuring
KindeClientSessionImpl.login(" ") preserves the normal login flag while
dropping invitation_code and is_invitation.
In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java`:
- Around line 66-69: The current null-only check for req.getParameter(ORG_NAME)
allows whitespace-only org_name values to pass; update the validation in
KindeAuthenticationFilter so you trim the parameter (e.g., String orgName =
req.getParameter(ORG_NAME)) and reject it if it is null or empty after trimming
by throwing the same ServletException with a clear message, and then call
kindeClientSession.createOrg(orgName, invitationCode) using the validated
orgName.
---
Outside diff comments:
In
`@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java`:
- Around line 137-138: Tests currently encode state with
java.util.Base64.getEncoder() while production uses Base64.getUrlDecoder();
update the tests to use the URL-safe encoder by replacing getEncoder() with
java.util.Base64.getUrlEncoder() when building encodedState in the test methods
testLoginLinkExpiredRedirectsWithDecodedParams and
testLoginLinkExpiredWithQueryParamsInAuthUrl so the encoded state matches the
URL-safe decoder used in production.
In `@playground/kinde-management-example/pom.xml`:
- Around line 63-67: The SLF4J dependency block for org.slf4j:slf4j-simple
contains an empty <scope></scope>, which is invalid; remove the empty <scope>
element or replace it with an explicit valid scope (e.g., set
<scope>runtime</scope>) in the dependency declaration for org.slf4j:slf4j-simple
so Maven no longer emits build validation warnings.
---
Duplicate comments:
In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java`:
- Around line 54-55: In KindeAuthenticationFilter, rawInvitationCode is
currently checked for non-blank but not trimmed, so values like " inv123 " are
forwarded; replace the logic that sets invitationCode to first trim
rawInvitationCode (e.g., String trimmed = rawInvitationCode != null ?
rawInvitationCode.trim() : null) then set invitationCode to trimmed if trimmed
is not blank, otherwise null, using the existing INVITATION_CODE parameter
handling to ensure leading/trailing whitespace is removed before use.
In
`@kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java`:
- Around line 115-126: The test
resolveWithClientRegistrationIdWithoutInvitationCodeDoesNotAddParams currently
checks the absent-invitation case but misses blank/whitespace invitation values;
update this test (or add companion tests) to also set request parameters
"invitation_code" to "" and to a whitespace-only string (e.g., " "), call
resolver.resolve(request, "kinde") for each case, and assert authRequest is not
null and that authRequest.getAdditionalParameters().get("invitation_code") and
get("is_invitation") are null; reference the resolver.resolve(request, "kinde")
call and the assertions on authRequest.getAdditionalParameters() to locate where
to add these checks.
- Around line 34-46: The test resolveWithInvitationCodeAddsParams currently only
checks OAuth2AuthorizationRequest.getAdditionalParameters; update it to also
assert that the emitted authorization URI (from
OAuth2AuthorizationRequest.getAuthorizationRequestUri()) contains the invitation
parameters so they are sent on the wire: after obtaining authRequest from
resolver.resolve(request), assert authRequest.getAuthorizationRequestUri() is
non-null and contains both "invitation_code=inv_abc123" and "is_invitation=true"
(URL-encoded as needed) to ensure resolver.resolve(...) actually propagates
these into the redirect URI.
---
Nitpick comments:
In
`@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java`:
- Around line 202-226: The tests testEmptyInvitationCodeFallsThrough and
testWhitespaceOnlyInvitationCodeFallsThrough currently only verify
mockSession.login() is invoked; extend each to assert the full fallthrough
behavior by also verifying the redirect occurs (like in
testRedirectToAuthorizationWhenNoPrincipalOrAuthUrl): assert
response.sendRedirect(...) is called with mockAuthUrl.getUrl().toString() (or
verify filterChain.doFilter(...) is not invoked) after filter.doFilter(...), and
ensure you keep the existing when(mockSession.login()).thenReturn(mockAuthUrl)
setup so the expected redirect URL is available.
- Around line 185-200: The test testInvitationCodeOverridesExistingSession
should also assert that the session's AUTHORIZATION_URL is updated; add a
verification that session.setAttribute(AUTHORIZATION_URL, invitationAuthUrl) is
called after filter.doFilter(...) so the session state is validated (locate in
KindeAuthenticationFilterTest near the existing
verify(mockSession).login("inv_override") and verify(response).sendRedirect(...)
assertions).
In
`@kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java`:
- Around line 152-164: The test doesn't assert that POST_LOGIN_URL is saved to
the session during the register-with-invitation flow; after calling
servlet.doGet(request, response, KindeAuthenticationAction.REGISTER) add a
verification that the session setAttribute was called with POST_LOGIN_URL and
the passed URL (e.g. verify(session).setAttribute(POST_LOGIN_URL,
"http://example.com/dashboard")); place this alongside the existing
verify(session).setAttribute(AUTHORIZATION_URL, mockAuthUrl) assertion so the
post-registration redirect contract is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 509e45fa-5533-4a90-8822-a1d7748266c7
📒 Files selected for processing (13)
kinde-core/src/main/java/com/kinde/KindeClientSession.javakinde-core/src/main/java/com/kinde/session/KindeClientCodeSessionImpl.javakinde-core/src/main/java/com/kinde/session/KindeClientSessionImpl.javakinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.javakinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.javakinde-j2ee/src/main/java/com/kinde/servlet/KindeAuthenticationServlet.javakinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.javakinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.javakinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/Kinde.javakinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolver.javakinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.javaplayground/kinde-j2ee-app/pom.xmlplayground/kinde-management-example/pom.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- kinde-springboot/kinde-springboot-core/src/main/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolver.java
- playground/kinde-j2ee-app/pom.xml
- kinde-j2ee/src/main/java/com/kinde/servlet/KindeAuthenticationServlet.java
- kinde-core/src/main/java/com/kinde/session/KindeClientCodeSessionImpl.java
- kinde-core/src/main/java/com/kinde/KindeClientSession.java
kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.java
Show resolved
Hide resolved
kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java
Outdated
Show resolved
Hide resolved
…oder mismatch - Reject null/blank invitationCode in handleInvitation with IllegalArgumentException - Trim invitation codes in filter and servlet to strip leading/trailing whitespace - Validate org_name with isBlank() (not just null) in both invitation and non-invitation CREATE_ORG paths across filter and servlet - Fix Base64 encoder in filter tests to use getUrlEncoder() matching production's getUrlDecoder() - Add supports_reauth assertion to whitespace login test - Add whitespace-only test for two-arg resolver overload - Add POST_LOGIN_URL assertion to servlet register-with-invitation test - Add blank org_name validation tests for filter and servlet Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java (1)
249-263: Consider using assertThrows for more precise exception testing.The
@Test(expected = ...)annotation only verifies the exception type, not the message content. For debugging and ensuring the correct validation path is triggered, consider usingassertThrows(JUnit 5) orExpectedExceptionrule (JUnit 4) to also verify the exception message mentions "org_name".♻️ Example with JUnit 4 ExpectedException
import org.junit.Rule; import org.junit.rules.ExpectedException; `@Rule` public ExpectedException thrown = ExpectedException.none(); `@Test` public void testInvitationCodeOnCreateOrgMissingOrgNameThrows() throws Exception { thrown.expect(jakarta.servlet.ServletException.class); thrown.expectMessage("org_name"); // Verify message contains expected text when(request.getParameter("invitation_code")).thenReturn("inv_org_no_name"); when(request.getParameter("org_name")).thenReturn(null); filter.doFilter(request, response, filterChain, KindeAuthenticationAction.CREATE_ORG); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java` around lines 249 - 263, Replace the current `@Test`(expected = ...) style in testInvitationCodeOnCreateOrgMissingOrgNameThrows and testInvitationCodeOnCreateOrgBlankOrgNameThrows with an explicit exception assertion that also verifies the exception message mentions "org_name" (either JUnit 5's assertThrows or JUnit 4's ExpectedException rule); specifically, invoke filter.doFilter(request, response, filterChain, KindeAuthenticationAction.CREATE_ORG) inside the assertion and assert the thrown jakarta.servlet.ServletException's message contains "org_name" so the tests validate the correct validation path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@kinde-core/src/main/java/com/kinde/session/KindeClientSessionImpl.java`:
- Around line 269-277: The createOrg method in KindeClientSessionImpl accepts
orgName without validation; add defensive validation to ensure orgName is
non-null and not blank (after trimming) before building parameters—trim the
value, throw an IllegalArgumentException (or appropriate unchecked exception) if
invalid, and only put the trimmed org_name into parameters; update
createOrg(String orgName, String invitationCode) accordingly to prevent sending
invalid parameters to authorizationUrlWithParameters.
In
`@kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java`:
- Around line 137-165: Add a positive trim-normalization test to
KindeAuthenticationServletTest: create a new test that calls
servlet.doGet(request, response, KindeAuthenticationAction.LOGIN) with
request.getParameter("invitation_code") returning " inv_test123 " and
POST_LOGIN_URL returning a target URL, stub mockAuthUrl.getUrl() and
mockSession.login("inv_test123") to return mockAuthUrl, then verify
mockSession.login was called with the trimmed value, session attributes
AUTHORIZATION_URL and POST_LOGIN_URL were set, and response.sendRedirect uses
mockAuthUrl.getUrl().toString(); similarly add a corresponding REGISTER (or
createOrg) test where org_name = " TestOrg " and invitation_code = "
inv_test123 " and verify mockSession.register or
mockSession.createOrg("TestOrg","inv_test123") is called with trimmed values and
the same session/redirect assertions (repeat pattern applied to the tests around
testDoGet_Register... and the block referenced at 176-215).
- Around line 167-174: The test
KindeAuthenticationServletTest.testDoGet_InvitationCode_WithoutPostLoginUrl_Throws
uses JUnit4's `@Test`(expected=...) which won't run under JUnit 5; either add the
junit-vintage-engine dependency to enable JUnit4 tests or update the test to
JUnit 5 by replacing the `@Test`(expected=ServletException.class) style with
assertThrows(ServletException.class, () -> servlet.doGet(request, response,
KindeAuthenticationAction.LOGIN)); ensure you keep the same stubbing for
request.getParameter("invitation_code"), request.getParameter(POST_LOGIN_URL)
and request.getParameter("code") and update the other three affected tests
(lines noted) the same way if you choose migration.
---
Nitpick comments:
In
`@kinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.java`:
- Around line 249-263: Replace the current `@Test`(expected = ...) style in
testInvitationCodeOnCreateOrgMissingOrgNameThrows and
testInvitationCodeOnCreateOrgBlankOrgNameThrows with an explicit exception
assertion that also verifies the exception message mentions "org_name" (either
JUnit 5's assertThrows or JUnit 4's ExpectedException rule); specifically,
invoke filter.doFilter(request, response, filterChain,
KindeAuthenticationAction.CREATE_ORG) inside the assertion and assert the thrown
jakarta.servlet.ServletException's message contains "org_name" so the tests
validate the correct validation path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bf2ae3d6-02ac-4287-9cfb-7de4f975799a
📒 Files selected for processing (7)
kinde-core/src/main/java/com/kinde/session/KindeClientSessionImpl.javakinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.javakinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.javakinde-j2ee/src/main/java/com/kinde/servlet/KindeAuthenticationServlet.javakinde-j2ee/src/test/java/com/kinde/filter/KindeAuthenticationFilterTest.javakinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.javakinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- kinde-core/src/test/java/com/kinde/session/KindeClientSessionImplTest.java
- kinde-springboot/kinde-springboot-core/src/test/java/com/kinde/spring/KindeOAuth2AuthorizationRequestResolverTest.java
- kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java
- kinde-j2ee/src/main/java/com/kinde/servlet/KindeAuthenticationServlet.java
kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java
Show resolved
Hide resolved
kinde-j2ee/src/test/java/com/kinde/servlet/KindeAuthenticationServletTest.java
Show resolved
Hide resolved
…ception message assertions - Validate orgName for null/blank in KindeClientSessionImpl.createOrg and trim the value before adding to parameters, matching handleInvitation's guard - Add core-level tests for createOrg: null throws, blank throws, padded trims - Add servlet trim normalization tests verifying padded invitation codes and org names are trimmed before being forwarded - Replace @test(expected=...) with try-catch in filter org_name tests to also assert the exception message mentions "org_name" Made-with: Cursor
Invitation code functionality added
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.
Summary by CodeRabbit
New Features
Bug Fixes