Implement federated sign-in buttons and associated logic for Google and GitHub#3
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds redirect-based federated authentication continuation on Android, iOS, and Dart; introduces typed cancellation handling; extends flow response encoding; adds branded trigger buttons and SVG rendering; and updates ChangesFederated auth and flow rendering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SignIn
participant ThunderIDClient
participant NativeHandler
participant FederatedAuthSession
participant FlowServer
SignIn->>ThunderIDClient: continueFederatedAuth redirectURL
ThunderIDClient->>NativeHandler: invoke native continuation
NativeHandler->>FederatedAuthSession: authenticate callback
FederatedAuthSession-->>NativeHandler: authorization code or cancellation
NativeHandler->>FlowServer: signIn with embedded code payload
FlowServer-->>ThunderIDClient: encoded completed flow response
ThunderIDClient-->>SignIn: EmbeddedFlowResponse or typed error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
brionmario
force-pushed
the
feat-federated
branch
from
July 16, 2026 07:18
921999b to
43a6d71
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
test/adapters_test.dart (1)
26-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd a visual assertion for the branded glyphs.
A golden test would catch both malformed SVG geometry and an invisible white-on-light GitHub glyph; checking only labels and exceptions cannot detect either regression.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/adapters_test.dart` around lines 26 - 50, Add golden-image assertions to the GoogleButton and GitHubButton widget tests, capturing each rendered branded glyph and comparing it against approved golden files. Keep the existing label, exception, and tap assertions, and ensure the goldens visibly validate both SVG geometry and GitHub glyph contrast.test/flow_form_test.dart (1)
209-238: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the nested federated trigger actions in this fixture.
Assert that
GoogleButtonandGitHubButtonrender, then tap each and verify submission ofaction_xoc0andaction_zeye. The current tests ignore these fixture components and therefore pass while both triggers route to the first credential action.Also applies to: 240-280
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/flow_form_test.dart` around lines 209 - 238, Extend the tests around buildForm and the affected flow-form cases to find and assert both GoogleButton and GitHubButton, tap each trigger, and verify submit receives action_xoc0 and action_zeye respectively. Ensure the assertions exercise the nested federated trigger actions rather than only validating the fixture renders, and preserve existing coverage for other credential actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.kt`:
- Around line 74-76: Update onDetachedFromActivity to call cancelIfPending()
before or alongside detachActivity(), ensuring any suspended authentication
request is resolved when the host is permanently detached. Keep
onReattachedToActivityForConfigChanges unchanged so pending sessions remain
preserved across configuration changes.
In `@ios/Classes/ThunderIDMethodHandler.swift`:
- Around line 129-136: Remove the iOS-only rejection of missing afterSignInUrl
in the sign-in callback flow around callbackURLScheme; derive the callback
scheme from the authorization URL’s redirect_uri or pass it explicitly through
the channel instead. Preserve optional afterSignInUrl behavior consistently with
Android and the Dart API, and apply the same correction to the related logic
around the second referenced block.
In `@lib/src/widgets/adapters/github_button.dart`:
- Around line 33-42: Update the GitHub glyph color in the MultiColorSvgIcon
within the build method to use the current theme’s contrasting or foreground
color instead of the hard-coded white Color value, ensuring it remains visible
on the default light button surface.
In `@lib/src/widgets/adapters/svg_icon_path.dart`:
- Around line 189-210: Update the path application logic alongside
_argumentCount to implement smooth cubic S/s commands instead of discarding
their parsed arguments. Track the previous cubic control point, reflect it for
each S/s command when the prior segment is cubic, and update the current
point/control state so subsequent relative segments retain correct geometry;
preserve existing handling for explicit C/c and other commands.
In `@lib/src/widgets/flow_form.dart`:
- Around line 289-313: Update the HTML-to-spans logic around _stripTags in the
link parsing loop and trailing-text handling so whitespace surrounding inline
anchors is preserved; avoid trimming the plain-text segments before adding them,
while retaining empty-segment filtering and existing link recognizer behavior.
- Around line 374-405: Update the trigger handling in the flow component so
nested ACTION/BLOCK wrappers are unwrapped and their child trigger components
are resolved before determining the action and button. Ensure identifiers such
as action_xoc0 and action_zeye are selected from the nested children rather than
block_per3/block_dzuu or the first action, while preserving the existing
provider-specific button rendering and submitting the resolved trigger action.
- Line 58: Update _renderRichText and the _linkRecognizers field so
TapGestureRecognizer instances are reused by href across rebuilds, or relocate
link rendering into a dedicated stateful widget that disposes recognizers when
replaced; ensure obsolete recognizers are disposed and retain disposal during
form teardown.
- Around line 330-335: Update _openLink to validate the parsed URI scheme before
calling launchUrl, allowing only http and https schemes; return without
launching for all other schemes while preserving the existing empty and
invalid-URL checks.
---
Nitpick comments:
In `@test/adapters_test.dart`:
- Around line 26-50: Add golden-image assertions to the GoogleButton and
GitHubButton widget tests, capturing each rendered branded glyph and comparing
it against approved golden files. Keep the existing label, exception, and tap
assertions, and ensure the goldens visibly validate both SVG geometry and GitHub
glyph contrast.
In `@test/flow_form_test.dart`:
- Around line 209-238: Extend the tests around buildForm and the affected
flow-form cases to find and assert both GoogleButton and GitHubButton, tap each
trigger, and verify submit receives action_xoc0 and action_zeye respectively.
Ensure the assertions exercise the nested federated trigger actions rather than
only validating the fixture renders, and preserve existing coverage for other
credential actions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac8ddb26-ff01-4385-9396-2bc3048d7fbe
📒 Files selected for processing (15)
android/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.ktandroid/src/main/kotlin/dev/thunderid/flutter/ThunderIDMethodHandler.ktios/Classes/ThunderIDMethodHandler.swiftlib/src/models/thunderid_error.dartlib/src/thunderid_client.dartlib/src/widgets/adapters/github_button.dartlib/src/widgets/adapters/google_button.dartlib/src/widgets/adapters/outlined_trigger_button.dartlib/src/widgets/adapters/svg_icon_path.dartlib/src/widgets/flow_form.dartlib/src/widgets/thunderid_sign_in.dartpubspec.yamltest/adapters_test.darttest/flow_form_test.darttest/thunderid_client_test.dart
brionmario
force-pushed
the
feat-federated
branch
from
July 16, 2026 11:17
43a6d71 to
1ae971c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
android/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.kt (1)
74-76: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCancel pending authentication on permanent Activity detach.
If the host is permanently detached while the Custom Tab is open, it never resumes and
cancelIfPending()is never called. The suspended method-channel request can therefore remain unresolved indefinitely. Cancel here, while preserving the session during configuration changes.Proposed fix
override fun onDetachedFromActivity() { + FederatedAuthSession.cancelIfPending() detachActivity() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.kt` around lines 74 - 76, Update onDetachedFromActivity to cancel any pending authentication via cancelIfPending() before or while calling detachActivity(). Preserve the existing configuration-change behavior by leaving onReattachedToActivityForConfigChanges and related temporary-detach handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/build.gradle`:
- Around line 64-66: Update the android-sdks dependency declaration in the
Gradle dependencies block to use a stable, immutable release such as v0.1.0
instead of main-SNAPSHOT, while leaving the other dependencies unchanged.
In `@lib/src/widgets/sign_in.dart`:
- Around line 143-145: Update the debug log in the sign-in redirection flow
around continueFederatedAuth to avoid hardcoding “ThunderID” in the tag. Use the
configured vendor value from the existing client/config context if a
brand-specific tag is required; otherwise remove the vendor prefix while
preserving the actionId and delegation details.
---
Duplicate comments:
In `@android/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.kt`:
- Around line 74-76: Update onDetachedFromActivity to cancel any pending
authentication via cancelIfPending() before or while calling detachActivity().
Preserve the existing configuration-change behavior by leaving
onReattachedToActivityForConfigChanges and related temporary-detach handling
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bc1ec0f8-068a-45e4-bc4e-49fa81e4da28
📒 Files selected for processing (20)
android/build.gradleandroid/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.ktandroid/src/main/kotlin/dev/thunderid/flutter/ThunderIDMethodHandler.ktios/Classes/ThunderIDMethodHandler.swiftlib/src/models/thunderid_error.dartlib/src/thunderid_client.dartlib/src/widgets/adapters/facebook_button.dartlib/src/widgets/adapters/github_button.dartlib/src/widgets/adapters/google_button.dartlib/src/widgets/adapters/linkedin_button.dartlib/src/widgets/adapters/microsoft_button.dartlib/src/widgets/adapters/outlined_trigger_button.dartlib/src/widgets/adapters/svg_icon_path.dartlib/src/widgets/flow_form.dartlib/src/widgets/sign_in.dartpubspec.yamlsamples/quickstart/ios/Podfiletest/adapters_test.darttest/flow_form_test.darttest/thunderid_client_test.dart
🚧 Files skipped from review as they are similar to previous changes (11)
- lib/src/widgets/adapters/google_button.dart
- pubspec.yaml
- lib/src/thunderid_client.dart
- test/adapters_test.dart
- lib/src/widgets/adapters/github_button.dart
- test/flow_form_test.dart
- lib/src/widgets/adapters/outlined_trigger_button.dart
- lib/src/widgets/adapters/svg_icon_path.dart
- test/thunderid_client_test.dart
- ios/Classes/ThunderIDMethodHandler.swift
- lib/src/widgets/flow_form.dart
brionmario
force-pushed
the
feat-federated
branch
2 times, most recently
from
July 16, 2026 12:15
6ff4371 to
bedb79c
Compare
brionmario
added a commit
to brionmario/flutter-sdks
that referenced
this pull request
Jul 16, 2026
- Cancel a pending federated-auth Custom Tab session on permanent Activity detach (Android), not just on config-change detach, so a suspended method-channel request can't hang indefinitely. - Derive the iOS ASWebAuthenticationSession callback scheme from the authorization URL's redirect_uri first, falling back to the optional afterSignInUrl only if that's absent — federated sign-in no longer requires afterSignInUrl on iOS when Android and the Dart API don't. - Implement S/s (smooth cubic) SVG path commands in the icon path parser; the GitHub glyph path uses them and they were silently dropped, corrupting downstream relative coordinates. - Preserve whitespace around inline rich-text links instead of trimming it away, and restrict rich-text link taps to http/https schemes. - Dispose stale TapGestureRecognizers before each rebuild instead of only at widget teardown, so rebuilds don't accumulate them. - Resolve the SignIn debug log tag from ThunderIDConfig.vendor instead of a hardcoded "ThunderID" literal, consistent with the SDK's white-labeling rules. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/src/main/kotlin/dev/thunderid/flutter/ThunderIDMethodHandler.kt`:
- Around line 131-134: Update the CancellationException catch block in
ThunderIDMethodHandler so it reports FEDERATED_AUTH_CANCELLED through
result.error, then rethrows the same exception to preserve coroutine
cancellation propagation and structured concurrency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b2d0102e-9654-4bbc-8d08-b922da83ebac
📒 Files selected for processing (21)
android/build.gradleandroid/src/main/kotlin/dev/thunderid/flutter/ThunderIDFlutterPlugin.ktandroid/src/main/kotlin/dev/thunderid/flutter/ThunderIDMethodHandler.ktios/Classes/ThunderIDMethodHandler.swiftlib/src/models/thunderid_error.dartlib/src/thunderid_client.dartlib/src/widgets/adapters/facebook_button.dartlib/src/widgets/adapters/github_button.dartlib/src/widgets/adapters/google_button.dartlib/src/widgets/adapters/linkedin_button.dartlib/src/widgets/adapters/microsoft_button.dartlib/src/widgets/adapters/outlined_trigger_button.dartlib/src/widgets/adapters/svg_icon_path.dartlib/src/widgets/flow_form.dartlib/src/widgets/sign_in.dartpubspec.yamlsamples/quickstart/android/build.gradle.ktssamples/quickstart/ios/Podfiletest/adapters_test.darttest/flow_form_test.darttest/thunderid_client_test.dart
🚧 Files skipped from review as they are similar to previous changes (17)
- lib/src/widgets/adapters/github_button.dart
- lib/src/widgets/adapters/google_button.dart
- pubspec.yaml
- lib/src/thunderid_client.dart
- lib/src/widgets/adapters/linkedin_button.dart
- test/adapters_test.dart
- test/thunderid_client_test.dart
- samples/quickstart/ios/Podfile
- lib/src/widgets/adapters/facebook_button.dart
- test/flow_form_test.dart
- lib/src/widgets/adapters/microsoft_button.dart
- lib/src/widgets/adapters/outlined_trigger_button.dart
- lib/src/models/thunderid_error.dart
- lib/src/widgets/sign_in.dart
- ios/Classes/ThunderIDMethodHandler.swift
- lib/src/widgets/adapters/svg_icon_path.dart
- lib/src/widgets/flow_form.dart
Comment on lines
+131
to
+134
| } catch (e: CancellationException) { | ||
| // User dismissed the Custom Tab without completing federated sign-in — surfaced as a | ||
| // typed cancellation so the Dart layer can reset state silently instead of erroring. | ||
| result.error("FEDERATED_AUTH_CANCELLED", "User cancelled federated sign-in", null) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Rethrow CancellationException to maintain structured concurrency.
When catching CancellationException in Kotlin coroutines, it is best practice to rethrow it after handling (such as sending the error via the MethodChannel). Swallowing it prevents the coroutine machinery from properly propagating the cancellation to the parent scope. As per static analysis hints, the caught exception is swallowed, which could lead to the original exception being lost.
🛠️ Proposed fix to rethrow the exception
} catch (e: CancellationException) {
// User dismissed the Custom Tab without completing federated sign-in — surfaced as a
// typed cancellation so the Dart layer can reset state silently instead of erroring.
result.error("FEDERATED_AUTH_CANCELLED", "User cancelled federated sign-in", null)
+ throw e
} catch (e: Exception) {📝 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
| } catch (e: CancellationException) { | |
| // User dismissed the Custom Tab without completing federated sign-in — surfaced as a | |
| // typed cancellation so the Dart layer can reset state silently instead of erroring. | |
| result.error("FEDERATED_AUTH_CANCELLED", "User cancelled federated sign-in", null) | |
| } catch (e: CancellationException) { | |
| // User dismissed the Custom Tab without completing federated sign-in — surfaced as a | |
| // typed cancellation so the Dart layer can reset state silently instead of erroring. | |
| result.error("FEDERATED_AUTH_CANCELLED", "User cancelled federated sign-in", null) | |
| throw e |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 131-131: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/src/main/kotlin/dev/thunderid/flutter/ThunderIDMethodHandler.kt`
around lines 131 - 134, Update the CancellationException catch block in
ThunderIDMethodHandler so it reports FEDERATED_AUTH_CANCELLED through
result.error, then rethrows the same exception to preserve coroutine
cancellation propagation and structured concurrency.
Source: Linters/SAST tools
brionmario
force-pushed
the
feat-federated
branch
from
July 16, 2026 12:36
108a3b6 to
b2ec88f
Compare
JayaShakthi97
approved these changes
Jul 16, 2026
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
2 participants
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.
Purpose
Implement Google & GitHub buttons.
Approach
N/A
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests