feat: Add Connection ID support across all frameworks#212
Open
BrandtKruger wants to merge 4 commits intomainfrom
Open
feat: Add Connection ID support across all frameworks#212BrandtKruger wants to merge 4 commits intomainfrom
BrandtKruger wants to merge 4 commits intomainfrom
Conversation
- 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 across core and J2EE modules: new constants, token API and BaseToken implementation to extract connection_id (direct or nested), propagation of connection_id into authorization URL generation in filters/servlets, and corresponding unit tests and JWT test helpers. 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: HTTP request (may include connection_id)
Filter->>Filter: extract params map (connection_id, org_code, lang, etc.)
alt params present
Filter->>Session: authorizationUrlWithParameters(action, params)
else no params
Filter->>Session: login()/register()/createOrg() (legacy)
end
Session->>Auth: generate authorization URL (includes params if provided)
Auth->>Filter: return authorization URL
Filter->>Client: redirect to authorization URL
Client->>Auth: completes auth, receives token (JWT)
Client->>Token: parse JWT
Token->>Token: getConnectionId() → check `connection_id` or `ext_provider.connection_id`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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(); }
kinde-core/src/test/java/com/kinde/session/ConnectionIdTest.java
Outdated
Show resolved
Hide resolved
kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java
Show resolved
Hide resolved
kinde-j2ee/src/main/java/com/kinde/filter/KindeAuthenticationFilter.java
Outdated
Show resolved
Hide resolved
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! |
BrandtKruger
force-pushed
the
feature/connection-id-support
branch
from
4c5f15b to
7275c40
Compare
- Change State.ACTIVE to State.TEST in ConnectionIdTest for consistency - Fix testGetConnectionIdPreferDirectOverNested to properly test preference - Add generateIDTokenWithBothConnectionIds() method to JwtGenerator - Test now verifies direct connection_id is preferred over nested - Fix KindeAuthenticationFilter and KindeAuthenticationServlet: - Add missing supports_reauth=true for REGISTER action - Add missing is_create_org=true for CREATE_ORG action - Fix typo: 'proved' -> 'provided' - Ensure parameter preservation matches dedicated method behavior
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@kinde-core/src/test/java/com/kinde/token/ConnectionIdTokenTest.java`:
- Around line 111-124: The test method
testGetConnectionIdWithExtProviderButNoConnectionId is mislabeled because
JwtGenerator.generateIDToken() produces a token without ext_provider; either
update the `@DisplayName` to reflect "ext_provider is null" or change the token
generation to exercise the intended case: add a new JwtGenerator method (e.g.,
generateIDTokenWithExtProviderNoConnectionId) that builds a token containing an
ext_provider object without a connection_id, call that new generator in
testGetConnectionIdWithExtProviderButNoConnectionId, and keep the existing
assertions against KindeToken (IDToken.init and getConnectionId()) to verify
null is returned.
🧹 Nitpick comments (1)
kinde-j2ee/src/main/java/com/kinde/servlet/KindeAuthenticationServlet.java (1)
68-100: Consider extracting shared authorization URL building logic.The parameter building and action handling logic (lines 68-100) is nearly identical to
KindeAuthenticationFilter(lines 70-102). Consider extracting this to a shared utility class to reduce duplication and ensure consistent behavior.
…operly test ext_provider without connection_id - Add generateIDTokenWithExtProviderButNoConnectionId() method to JwtGenerator - Creates token with ext_provider containing other fields but no connection_id - Test now properly verifies the edge case where ext_provider exists but lacks connection_id - Fixes CodeRabbit comment about test description not matching behavior
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.