feat: Add Connection ID support across all frameworks #212
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add CONNECTION_ID constant to KindeRequestParameters and KindeConstants - Implement getConnectionId() method in KindeToken interface and BaseToken - Supports direct connection_id claim - Supports nested ext_provider.connection_id structure - Add connection_id parameter support to authorizationUrlWithParameters() - Update J2EE filter and servlet to read connection_id from request parameters - Works with LOGIN, REGISTER, and CREATE_ORG actions - Add comprehensive test coverage: - ConnectionIdTest: 5 tests for authorization URL generation - ConnectionIdTokenTest: 8 tests for token extraction - ConnectionIdFilterTest: J2EE filter integration tests - Add JWT generator helpers for testing connection_id claims This implementation enables: - Passing connection_id when generating authorization URLs for social/enterprise login - Extracting connection_id from tokens (both direct and nested structures) - Automatic connection_id support in J2EE via request parameters - Core support available for SpringBoot (requires Spring Security config) All changes are backward compatible and optional.
Contributor
📝 WalkthroughWalkthroughAdds connection_id support: new constants, token extraction API and implementation, authorization URL propagation in filters/servlets, and unit tests plus test JWT helpers to validate behavior. Changes
Sequence DiagramsequenceDiagram
participant Client as HTTP Client
participant Filter as KindeAuthenticationFilter / Servlet
participant Session as KindeClientSession
participant Auth as AuthorizationUrl Generator
participant Token as KindeToken
Client->>Filter: Request (may include connection_id)
Filter->>Filter: Extract connection_id, build params map
alt connection_id Present
Filter->>Session: authorizationUrlWithParameters(action, params)
else No connection_id
Filter->>Session: login(action)
end
Session->>Auth: Generate authorization URL (includes params)
Auth->>Filter: Return URL
Filter->>Client: Redirect to authorization URL
Client->>Auth: Complete auth, receive token
Client->>Token: Parse token
Token->>Token: getConnectionId() -> check claim or ext_provider.connection_id
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
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@kinde-core/src/test/java/com/kinde/session/ConnectionIdTest.java`:
- Line 27: The test currently calls
KindeEnvironmentSingleton.init(KindeEnvironmentSingleton.State.ACTIVE); to set
environment state; change this to
KindeEnvironmentSingleton.init(KindeEnvironmentSingleton.State.TEST); to align
with other unit tests (KindClientBuilderTest, KindTokenFactoryImplTest,
KindClientImplTest) unless ACTIVE is intentionally required—if ACTIVE is
intentional, add a brief comment above the call explaining why ACTIVE-specific
behavior is needed for ConnectionIdTest to justify the deviation.
In `@kinde-core/src/test/java/com/kinde/token/ConnectionIdTokenTest.java`:
- Around line 52-67: Update the test testGetConnectionIdPreferDirectOverNested
to actually verify precedence by creating a token that contains both a top-level
connection_id and a nested ext_provider.connection_id with different values; add
a helper in JwtGenerator (e.g., generateIDTokenWithBothConnectionIds(String
direct, String nested)) that injects both claims, then call
IDToken.init(tokenString, true) and assert KindeToken.getConnectionId() equals
the directConnectionId, ensuring the direct claim is preferred over
ext_provider.connection_id.
In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java`:
- Line 88: In KindeAuthenticationFilter replace the typo in the thrown
ServletException message—change "Must proved org_name query parameter to create
an organisation." to use "provided" so the exception reads something like "Must
provide org_name query parameter to create an organisation."; update the string
in the throw statement inside KindeAuthenticationFilter to correct the wording.
- Around line 70-97: The REGISTER and CREATE_ORG branches build parameter maps
for authorizationUrlWithParameters but omit parameters that the dedicated
methods add; update the REGISTER branch to add
registerParams.put("supports_reauth", "true") alongside prompt=create (so it
matches register()), and update the CREATE_ORG branch to add
createOrgParams.put("is_create_org", "true") alongside prompt=create and
org_name (so it matches createOrg()); modify the blocks that construct
registerParams and createOrgParams in KindeAuthenticationFilter (the branches
handling KindeAuthenticationAction.REGISTER and
KindeAuthenticationAction.CREATE_ORG and the calls to
kindeClientSession.authorizationUrlWithParameters(...)) to include those keys.
In `@kinde-j2ee/src/test/java/com/kinde/filter/ConnectionIdFilterTest.java`:
- Around line 83-116: The tests fail because KindeSingleton.setInstance(...)
does not exist; either add a static test-only setter on KindeSingleton (e.g.,
public/static void setInstance(KindeSingleton instance)) and a reset method
(clearInstance or setInstance(null)) so ConnectionIdFilterTest can inject and
clean up the singleton, or update ConnectionIdFilterTest to avoid modifying
global state (use dependency injection or obtain the singleton via a
factory/mocking approach or use reflection to set the private static field).
Ensure the change exposes/uses the symbols referenced in the test
(KindeSingleton.setInstance, KindeSingleton) and add cleanup after each test to
reset the singleton.
🧹 Nitpick comments (3)
kinde-core/src/main/java/com/kinde/token/BaseToken.java (1)
93-118: Use shared constant forconnection_idlookups.
Avoids drift between token claims and parameter keys.♻️ Suggested refactor
+import com.kinde.constants.KindeConstants; @@ - Object connectionId = getClaim("connection_id"); + Object connectionId = getClaim(KindeConstants.CONNECTION_ID); @@ - Object nestedConnectionId = extProviderMap.get("connection_id"); + Object nestedConnectionId = extProviderMap.get(KindeConstants.CONNECTION_ID);kinde-core/src/test/java/com/kinde/token/ConnectionIdTokenTest.java (1)
97-124: Tests 7 and 8 cover identical scenarios.Both
testGetConnectionIdWithNullExtProviderandtestGetConnectionIdWithExtProviderButNoConnectionIduseJwtGenerator.generateIDToken(), which produces a token without anyext_providerorconnection_idclaims. To test the described behavior in test 8 (ext_provider present but without connection_id), you'd need a token with anext_providerobject that has other properties but lacksconnection_id.💡 Suggested approach
Add a JwtGenerator method like
generateIDTokenWithEmptyExtProvider()orgenerateIDTokenWithExtProviderOtherKeys()that creates anext_providerobject withoutconnection_id, then use it intestGetConnectionIdWithExtProviderButNoConnectionId.kinde-core/src/test/java/com/kinde/session/ConnectionIdTest.java (1)
23-32: Consider adding@AfterEachcleanup to prevent test pollution.The
@BeforeEachcallsfin()theninit()on the singletons, which works for sequential tests, but adding explicit cleanup improves test isolation and makes the intent clearer.💡 Suggested addition
`@AfterEach` public void tearDown() { KindeGuiceSingleton.fin(); KindeEnvironmentSingleton.fin(); }
| public void setUp() { | ||
| KindeGuiceSingleton.fin(); | ||
| KindeEnvironmentSingleton.fin(); | ||
| KindeEnvironmentSingleton.init(KindeEnvironmentSingleton.State.ACTIVE); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for how State.ACTIVE vs State.TEST affects behavior
rg -n -B2 -A5 'getState\(\)' --type java | head -100
rg -n 'State\.(TEST|ACTIVE)' --type java | head -50Repository: kinde-oss/kinde-java-sdk
Length of output: 3785
🏁 Script executed:
#!/bin/bash
# Read the ConnectionIdTest.java file to understand the test context
cat -n kinde-core/src/test/java/com/kinde/session/ConnectionIdTest.javaRepository: kinde-oss/kinde-java-sdk
Length of output: 8666
Clarify the use of ACTIVE state; consider using TEST for consistency with other test classes.
This test uses State.ACTIVE while other similar tests (KindClientBuilderTest, KindTokenFactoryImplTest, KindClientImplTest) use State.TEST. The test doesn't appear to require ACTIVE-specific behavior and uses test modules. If ACTIVE is intentional, document the reason; otherwise, align with the established pattern of using TEST state in unit tests.
🤖 Prompt for AI Agents
In `@kinde-core/src/test/java/com/kinde/session/ConnectionIdTest.java` at line 27,
The test currently calls
KindeEnvironmentSingleton.init(KindeEnvironmentSingleton.State.ACTIVE); to set
environment state; change this to
KindeEnvironmentSingleton.init(KindeEnvironmentSingleton.State.TEST); to align
with other unit tests (KindClientBuilderTest, KindTokenFactoryImplTest,
KindClientImplTest) unless ACTIVE is intentionally required—if ACTIVE is
intentional, add a brief comment above the call explaining why ACTIVE-specific
behavior is needed for ConnectionIdTest to justify the deviation.
Comment on lines
+52
to
+67
| @Test | ||
| @DisplayName("getConnectionId should prefer direct connection_id over nested ext_provider.connection_id") | ||
| public void testGetConnectionIdPreferDirectOverNested() throws Exception { | ||
| // Create a token with direct connection_id | ||
| String directConnectionId = "conn_direct_123"; | ||
|
|
||
| // For this test, we'll use the direct one and verify it's preferred | ||
| String tokenString = JwtGenerator.generateIDTokenWithConnectionId(directConnectionId); | ||
|
|
||
| KindeToken kindeToken = IDToken.init(tokenString, true); | ||
|
|
||
| assertNotNull(kindeToken); | ||
| assertTrue(kindeToken.valid()); | ||
| assertEquals(directConnectionId, kindeToken.getConnectionId(), | ||
| "getConnectionId() should prefer direct connection_id claim"); | ||
| } |
Contributor
There was a problem hiding this comment.
Test doesn't verify preference behavior as described.
The test name and @DisplayName indicate it should verify that direct connection_id is preferred over nested ext_provider.connection_id, but it only creates a token with a direct claim. To properly test precedence, you need a token containing both claims with different values.
Consider adding a JwtGenerator method that creates a token with both claims, then verify the direct one wins:
💡 Example test approach
`@Test`
`@DisplayName`("getConnectionId should prefer direct connection_id over nested ext_provider.connection_id")
public void testGetConnectionIdPreferDirectOverNested() throws Exception {
String directConnectionId = "conn_direct_123";
String nestedConnectionId = "conn_nested_456";
// Need a new JwtGenerator method that adds BOTH claims
String tokenString = JwtGenerator.generateIDTokenWithBothConnectionIds(
directConnectionId, nestedConnectionId);
KindeToken kindeToken = IDToken.init(tokenString, true);
assertEquals(directConnectionId, kindeToken.getConnectionId(),
"getConnectionId() should prefer direct connection_id over ext_provider.connection_id");
}🤖 Prompt for AI Agents
In `@kinde-core/src/test/java/com/kinde/token/ConnectionIdTokenTest.java` around
lines 52 - 67, Update the test testGetConnectionIdPreferDirectOverNested to
actually verify precedence by creating a token that contains both a top-level
connection_id and a nested ext_provider.connection_id with different values; add
a helper in JwtGenerator (e.g., generateIDTokenWithBothConnectionIds(String
direct, String nested)) that injects both claims, then call
IDToken.init(tokenString, true) and assert KindeToken.getConnectionId() equals
the directConnectionId, ensuring the direct claim is preferred over
ext_provider.connection_id.
Comment on lines
70
to
+97
| if (kindeAuthenticationAction == KindeAuthenticationAction.LOGIN) { | ||
| authorizationUrl = kindeClientSession.login(); | ||
| if (parameters.isEmpty()) { | ||
| authorizationUrl = kindeClientSession.login(); | ||
| } else { | ||
| Map<String, String> loginParams = new HashMap<>(parameters); | ||
| loginParams.put("supports_reauth", "true"); | ||
| authorizationUrl = kindeClientSession.authorizationUrlWithParameters(loginParams); | ||
| } | ||
| } else if (kindeAuthenticationAction == KindeAuthenticationAction.REGISTER) { | ||
| authorizationUrl = kindeClientSession.register(); | ||
| if (parameters.isEmpty()) { | ||
| authorizationUrl = kindeClientSession.register(); | ||
| } else { | ||
| Map<String, String> registerParams = new HashMap<>(parameters); | ||
| registerParams.put("prompt", "create"); | ||
| authorizationUrl = kindeClientSession.authorizationUrlWithParameters(registerParams); | ||
| } | ||
| } else if (kindeAuthenticationAction == KindeAuthenticationAction.CREATE_ORG) { | ||
| if (req.getParameter(ORG_NAME) == null) { | ||
| throw new ServletException("Must proved org_name query parameter to create an organisation."); | ||
| } | ||
| authorizationUrl = kindeClientSession.createOrg(req.getParameter(ORG_NAME)); | ||
| if (parameters.isEmpty()) { | ||
| authorizationUrl = kindeClientSession.createOrg(req.getParameter(ORG_NAME)); | ||
| } else { | ||
| Map<String, String> createOrgParams = new HashMap<>(parameters); | ||
| createOrgParams.put("prompt", "create"); | ||
| createOrgParams.put("org_name", req.getParameter(ORG_NAME)); | ||
| authorizationUrl = kindeClientSession.authorizationUrlWithParameters(createOrgParams); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cd /tmp || exit 1
# Find the file
find . -name "*.java" -path "*KindeAuthenticationFilter*" 2>/dev/null | head -5Repository: kinde-oss/kinde-java-sdk
Length of output: 192
🏁 Script executed:
#!/bin/bash
# Search for authorizationUrlWithParameters implementation
rg -n -A 25 'authorizationUrlWithParameters' --type java
# Also search for login, register, createOrg implementations
rg -n -A 15 'public.*login\(\)' --type java | head -50
rg -n -A 15 'public.*register\(\)' --type java | head -50
rg -n -A 15 'public.*createOrg\(' --type java | head -50Repository: kinde-oss/kinde-java-sdk
Length of output: 49664
Ensure dedicated method parameters are preserved when using authorizationUrlWithParameters() in filter code.
While authorizationUrlWithParameters() does respect session configurations (grant type, scopes, redirect URI, etc.), the filter code has gaps in parameter preservation:
- REGISTER: The dedicated
register()method always addssupports_reauth=true, but the filter code only addsprompt=createwhen parameters exist—supports_reauthis not guaranteed. - CREATE_ORG: The dedicated
createOrg()method includesis_create_org=true, but the filter code does not preserve this parameter.
Update the filter code to explicitly add these parameters:
- For REGISTER: Add
registerParams.put("supports_reauth", "true"); - For CREATE_ORG: Add
createOrgParams.put("is_create_org", "true");
This ensures behavioral equivalence with the dedicated methods.
🤖 Prompt for AI Agents
In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java`
around lines 70 - 97, The REGISTER and CREATE_ORG branches build parameter maps
for authorizationUrlWithParameters but omit parameters that the dedicated
methods add; update the REGISTER branch to add
registerParams.put("supports_reauth", "true") alongside prompt=create (so it
matches register()), and update the CREATE_ORG branch to add
createOrgParams.put("is_create_org", "true") alongside prompt=create and
org_name (so it matches createOrg()); modify the blocks that construct
registerParams and createOrgParams in KindeAuthenticationFilter (the branches
handling KindeAuthenticationAction.REGISTER and
KindeAuthenticationAction.CREATE_ORG and the calls to
kindeClientSession.authorizationUrlWithParameters(...)) to include those keys.
| } | ||
| } else if (kindeAuthenticationAction == KindeAuthenticationAction.CREATE_ORG) { | ||
| if (req.getParameter(ORG_NAME) == null) { | ||
| throw new ServletException("Must proved org_name query parameter to create an organisation."); |
Contributor
There was a problem hiding this comment.
Fix typo: "proved" → "provided".
📝 Suggested fix
- throw new ServletException("Must proved org_name query parameter to create an organisation.");
+ throw new ServletException("Must provide org_name query parameter to create an organisation.");📝 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.
Suggested change
| throw new ServletException("Must proved org_name query parameter to create an organisation."); | |
| throw new ServletException("Must provide org_name query parameter to create an organisation."); |
🤖 Prompt for AI Agents
In `@kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java` at
line 88, In KindeAuthenticationFilter replace the typo in the thrown
ServletException message—change "Must proved org_name query parameter to create
an organisation." to use "provided" so the exception reads something like "Must
provide org_name query parameter to create an organisation."; update the string
in the throw statement inside KindeAuthenticationFilter to correct the wording.
kinde-j2ee/src/test/java/com/kinde/filter/ConnectionIdFilterTest.java
Outdated
Show resolved
Hide resolved
…eSingleton - Replace non-existent setInstance() with Mockito.mockStatic() pattern - Match existing test patterns in KindeAuthenticationFilterTest - Fix import statement for CONNECTION_ID constant - Add proper tearDown() to close MockedStatic
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implementation enables:
All changes are backward compatible and optional.
Explain your changes
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.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.