feat(davinci): add SOCIAL_LOGIN_BUTTON IdP collector support (SDKS-5128)#59
feat(davinci): add SOCIAL_LOGIN_BUTTON IdP collector support (SDKS-5128)#59pingidentity-gaurav wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughAdds DaVinci social-login collectors and external IdP authorization across shared types, native bridges, form handling, the sample app, and related tests. It also adds DaVinci instance identity access, collector resolution, retry behavior, profile navigation, and iOS configuration entries. ChangesDaVinci external IdP flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DaVinciIdpField
participant DaVinciClientPanel
participant ExternalIdpClient
participant NativeBridge
DaVinciIdpField->>DaVinciClientPanel: onIdpAuthorize(collector)
DaVinciClientPanel->>ExternalIdpClient: authorizeForDaVinci(daVinci, index)
ExternalIdpClient->>NativeBridge: authorizeForDaVinci(davinciId, options, config)
NativeBridge-->>ExternalIdpClient: authorization result
ExternalIdpClient-->>DaVinciClientPanel: resolve or reject
DaVinciClientPanel->>DaVinciClientPanel: next({ collectors: [] })
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 Additional details and impacted files@@ Coverage Diff @@
## main #59 +/- ##
============================================
+ Coverage 71.41% 71.70% +0.29%
- Complexity 181 187 +6
============================================
Files 154 161 +7
Lines 18052 18890 +838
Branches 539 674 +135
============================================
+ Hits 12891 13546 +655
- Misses 5111 5271 +160
- Partials 50 73 +23
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/davinci/src/types/node.types.ts (1)
385-385: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: preserve literal autocomplete on
idpType.
'GOOGLE' | 'FACEBOOK' | 'APPLE' | stringcollapses tostringin TypeScript, so editors lose autocomplete for the known providers while still accepting any string. If keeping the union open is the goal,... | (string & {})retains both.♻️ Suggested change
- idpType: 'GOOGLE' | 'FACEBOOK' | 'APPLE' | string; + idpType: 'GOOGLE' | 'FACEBOOK' | 'APPLE' | (string & {});🤖 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 `@packages/davinci/src/types/node.types.ts` at line 385, Update the idpType declaration to preserve autocomplete for the known GOOGLE, FACEBOOK, and APPLE literals while continuing to accept arbitrary strings, using the open-string union pattern described in the review.packages/external-idp/android/src/main/java/com/pingidentity/rnexternalidp/RNPingExternalIdpCommon.kt (1)
317-425: 📐 Maintainability & Code Quality | 🔵 TrivialConsider extracting the shared authorize-boilerplate.
authorizeForDaVinciduplicates most ofauthorizeForJourney's control flow: the foreground-activity guard,parseCallbackIndexusage,scope.launchBridgewrapping, and theCancellationException/ClassNotFoundException/NoClassDefFoundErrorcatch structure are copy-pasted verbatim, differing only in the resolved collector type and success payload. A small helper (e.g.runIdpAuthorize(promise, errorCode) { ... }wrapping the guard + catch logic, with the resolve/collector-lookup passed as a lambda) would reduce this duplication and keep future error-handling changes in one place.
[medium_effort_and_medium_reward]🤖 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 `@packages/external-idp/android/src/main/java/com/pingidentity/rnexternalidp/RNPingExternalIdpCommon.kt` around lines 317 - 425, Extract the duplicated authorization flow from authorizeForDaVinci and authorizeForJourney into a shared helper, such as runIdpAuthorize, that owns the foreground-activity check, scope.launchBridge wrapper, and CancellationException/provider-SDK/general error handling. Pass the provider-specific collector resolution and authorization operation as lambdas, while preserving each method’s collector type and success payload behavior.
🤖 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 `@packages/davinci/android/build.gradle`:
- Around line 99-100: Make IdpCollector available at runtime for
DaVinciNodeMapper.mapCollectorPayload by changing the external-idp dependency
from compileOnly to the appropriate runtime-inclusive configuration. Preserve
the existing collector mapping behavior so apps using rn-davinci alone cannot
fail with a missing IdpCollector class.
In `@packages/external-idp/ios/RNPingExternalIdpCommon.swift`:
- Around line 302-304: The authorization flow in IdpCollector.authorize must
receive only the callback URL scheme, not the full redirect URI. Update the
callbackURLScheme construction near callConfig.redirectUri to parse and extract
its scheme while preserving nil for an empty redirect URI, then pass that value
to authorize.
In `@PingSampleApp/ios/PingSampleApp/Info.plist`:
- Around line 66-67: Update the GIDServerClientID entry in Info.plist to
reference the backend web/server OAuth client ID configuration, while preserving
GIDClientID as the iOS client ID. Use the existing backend client-ID symbol or
setting rather than GOOGLE_IOS_CLIENT_ID.
In `@PingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.ts`:
- Around line 73-94: Prevent idpJustAuthorized from leaking past the immediate
post-authorization ContinueNode: in
PingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.ts:73-94, call
onForwarded() when the ContinueNode cannot auto-forward due to manual input,
integration requirements, unsupported state, or !canSubmit, while preserving the
successful-forward path; in
PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts:122-131, reset
setIdpJustAuthorized(false) in onStart alongside setIdpError(null).
In `@PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts`:
- Around line 169-203: Update onIdpAuthorize to handle a missing collector match
explicitly instead of passing index 0 to authorizeForDaVinci. When findIndex
returns -1, surface an appropriate error through the existing IdP error handling
and return without authorizing; preserve the current authorization flow for
valid indices.
---
Nitpick comments:
In `@packages/davinci/src/types/node.types.ts`:
- Line 385: Update the idpType declaration to preserve autocomplete for the
known GOOGLE, FACEBOOK, and APPLE literals while continuing to accept arbitrary
strings, using the open-string union pattern described in the review.
In
`@packages/external-idp/android/src/main/java/com/pingidentity/rnexternalidp/RNPingExternalIdpCommon.kt`:
- Around line 317-425: Extract the duplicated authorization flow from
authorizeForDaVinci and authorizeForJourney into a shared helper, such as
runIdpAuthorize, that owns the foreground-activity check, scope.launchBridge
wrapper, and CancellationException/provider-SDK/general error handling. Pass the
provider-specific collector resolution and authorization operation as lambdas,
while preserving each method’s collector type and success payload behavior.
🪄 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: 73a3afb5-4ff0-4d69-9705-98887b6e606e
⛔ Files ignored due to path filters (1)
PingSampleApp/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
.gitignorePingSampleApp/ios/PingSampleApp/Info.plistPingSampleApp/metro.config.jsPingSampleApp/ui/DaVinciScreen.tsxPingSampleApp/ui/davinci/components/molecules/DaVinciFieldRenderer.tsxPingSampleApp/ui/davinci/components/molecules/DaVinciIdpField.tsxPingSampleApp/ui/davinci/components/molecules/types.tsPingSampleApp/ui/davinci/components/organisms/DaVinciClientPanel.tsxPingSampleApp/ui/davinci/components/organisms/DaVinciContinueNodePanel.tsxPingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.tsPingSampleApp/ui/davinci/hooks/useDaVinciAutoStartEffect.tsPingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.tsPingSampleApp/ui/userProfile/components/molecules/UserProfileInfoCard.tsxPingTestRunner/__tests__/integration/external-idp.test.tsPingTestRunner/android/settings.gradlepackages/core/ios/CoreRuntime.swiftpackages/davinci/RNPingDavinci.podspecpackages/davinci/android/build.gradlepackages/davinci/android/src/main/java/com/pingidentity/rndavinci/mapper/DaVinciNodeMapper.ktpackages/davinci/android/src/test/java/com/pingidentity/rndavinci/mapper/DaVinciNodeMapperTest.ktpackages/davinci/ios/Mapper/DaVinciNodeMapper.swiftpackages/davinci/ios/RNPingDavinciCommon.swiftpackages/davinci/ios/Tests/DaVinciNodeMapperTests.swiftpackages/davinci/src/__tests__/collectorHelpers.test.tspackages/davinci/src/__tests__/useDavinciForm.test.tsxpackages/davinci/src/collectorHelpers.tspackages/davinci/src/davinci.tspackages/davinci/src/index.tsxpackages/davinci/src/types/client.types.tspackages/davinci/src/types/node.types.tspackages/davinci/src/useDavinci.tsxpackages/external-idp/README.mdpackages/external-idp/android/build.gradlepackages/external-idp/android/src/main/java/com/pingidentity/rnexternalidp/RNPingExternalIdpCommon.ktpackages/external-idp/android/src/newarch/java/com/pingidentity/rnexternalidp/RNPingExternalIdpModule.ktpackages/external-idp/android/src/oldarch/java/com/pingidentity/rnexternalidp/RNPingExternalIdpClassicModule.ktpackages/external-idp/android/src/test/java/com/pingidentity/rnexternalidp/RNPingExternalIdpTest.ktpackages/external-idp/ios/RNPingExternalIdp.mmpackages/external-idp/ios/RNPingExternalIdpClassic.mmpackages/external-idp/ios/RNPingExternalIdpCommon.swiftpackages/external-idp/ios/RNPingExternalIdpImpl.swiftpackages/external-idp/ios/Tests/RNPingExternalIdpImplTests.swiftpackages/external-idp/src/NativeRNPingExternalIdp.tspackages/external-idp/src/__tests__/index.test.tsxpackages/external-idp/src/externalIdp.tspackages/external-idp/src/types/externalIdp.types.tspackages/external-idp/src/types/index.tspackages/types/src/index.ts
| // Required: IdpCollector is defined in external-idp. The JS authorizeForDaVinci API is optional but this native dep is not. | ||
| compileOnly("com.pingidentity.sdks:external-idp:2.0.1") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the mapper references IdpCollector at runtime (unconditional type check)
rg -nP -C3 '\bis\s+IdpCollector\b|collector\.idpId' \
packages/davinci/android/src/main/java/com/pingidentity/rndavinci/mapper/DaVinciNodeMapper.kt
# Confirm external-idp is only compileOnly (not implementation/api) in the davinci module
rg -nP 'external-idp' packages/davinci/android/build.gradleRepository: ForgeRock/ping-react-native-sdk
Length of output: 1554
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files =="
git ls-files | rg '(^packages/davinci/|build\.gradle|Podspec$|package\.json$)' || true
echo
echo "== DaVinci Android build deps outline =="
awk '
/dependencies \{/,/^\}/ { print NR": "$0 }
' packages/davinci/android/build.gradle | sed 's/.*dependencies \{/dependencies {/; s/^\}\s*$/}/'
echo
echo "== All external-idp references =="
rg -n 'external-idp|external_idp|PingExternalIdP|IdpCollector|daVinci|DaVinci' .Repository: ForgeRock/ping-react-native-sdk
Length of output: 5601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l packages/davinci/android/build.gradle
echo
cat -n packages/davinci/android/build.gradle
echo
echo "== external-idp references in DaVinci =="
rg -n 'external-idp|external_idp|PingExternalIdP|IdpCollector|collectors|collector' packages/davinci -S
echo
echo "== DaVinci podspec dependency lines =="
rg -n 'dependency|vendored_libraries|s\.dependency|PingExternalIdP' packages/davinci/RNPingDavinci.podspec -A2 -B2
echo
echo "== external-idp Gradle and package artifacts =='
cat -n packages/external-idp/package.json
cat -n packages/external-idp/android/build.gradleRepository: ForgeRock/ping-react-native-sdk
Length of output: 50388
🌐 Web query:
Ping Identity react native SDK rn-davinci external-idp 2.0.1 IdpCollector darnci
💡 Result:
In the context of Ping Identity orchestration and DaVinci flows, IdpCollector is a component returned by a DaVinci flow to the client application when the user needs to authenticate using an external Identity Provider (IdP) [1][2]. The references to rn-davinci, 2.0.1, and darnci appear to be specific internal naming conventions or legacy terminology [3]. The Ping Identity documentation has moved toward a unified "Ping Orchestration SDKs 2.0" framework, which integrates support for DaVinci flows and replaces older modular SDK approaches [3]. Key technical points regarding IdpCollector and external IdPs: 1. Handling External IdPs: When a DaVinci flow returns an IdpCollector, the client application must use this collector to initiate the authentication process with the chosen external IdP [1][2]. 2. Implementation: For mobile platforms (like those using React Native), this typically involves calling an authorize method on the IdpCollector [2]. This initiates the redirect or embedded flow required to authenticate the user and return to the application via a continueToken [1]. 3. Architecture: The current recommendation is to use the Ping Orchestration SDKs, which provide unified documentation for JavaScript, Android, and iOS integrations [3]. Older documentation specifically labeled "DaVinci module" has been folded into this new Orchestration SDK documentation [3]. If you are encountering errors or specific behavior related to version 2.0.1 or terms like darnci, it is recommended to ensure your project is utilizing the current Ping Orchestration SDKs, as these are the primary supported libraries for DaVinci flow orchestration [4][3].
Citations:
- 1: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/javascript/02_authenticate_with_external_idps.html
- 2: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/03_authenticate_with_external_idps.html
- 3: https://developer.pingidentity.com/blog/ping-orchestration-sdks-2-0/
- 4: https://developer.pingidentity.com/sdks.html
🌐 Web query:
com.pingidentity.sdks external-idp 2.0.1 maven IdpCollector
💡 Result:
The artifact com.pingidentity.sdks:external-idp is a component of the Ping (ForgeRock) SDK suite used for integrating external identity providers (IdPs) into applications, particularly within DaVinci orchestration flows [1][2]. Regarding your specific query about version 2.0.1 and the IdpCollector class: 1. IdpCollector Usage: The IdpCollector is a node type returned by a DaVinci flow when an authentication process requires an external IdP [3][4][5]. Your application code must be configured to handle this collector node to initiate the authentication process (e.g., by calling authorize) [3][4]. 2. Dependency Information: While documentation references version 2.0.1 in the context of DaVinci Android SDK integration [1], publicly available artifacts on Maven Central for com.pingidentity.sdks:external-idp currently list version 1.2.0 as the latest [6]. If you are working in an environment that requires 2.0.1, ensure you are using the correct repository configuration provided by Ping Identity, as orchestration SDK dependencies are sometimes distributed through specialized channels [1]. 3. Documentation: For detailed implementation guidance, including class references and code samples for handling IdpCollector nodes in Android, iOS, and JavaScript, you should refer to the official Ping Identity Developer documentation for DaVinci SDKs [1][7][3][4][5].
Citations:
- 1: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/android/01_adding_core_dependencies.html
- 2: https://docs.pingidentity.com/sdks/latest/
- 3: https://docs.pingidentity.com/sdks/latest/davinci/use-cases/social-login/ios/03_handling_idpcollector_nodes.html
- 4: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/03_authenticate_with_external_idps.html
- 5: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/javascript/02_authenticate_with_external_idps.html
- 6: https://central.sonatype.com/artifact/com.pingidentity.sdks/external-idp
- 7: https://developer.pingidentity.com/reference/sdks/ios/unified/api-reference-2-0-0/ExternalIdP/Classes/IdpCollector.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Gradle GradleUtils compileOnly handling =="
rg -n 'compileOnly|implementation|api|transitive' --glob '*.*gradle*' .
echo
echo "== Kotlin type checks runtime semantics probe only if compiler available =="
if command -v kotlinc >/dev/null 2>&1; then
tmp="$(mktemp -d)"
cat > "$tmp/Main.kt" <<'KT'
sealed class C
object IdpCollector : C()
class Collector
fun C.isOrDoesWork(c: C): String = when(c) {
is IdpCollector -> "idp"
else -> "other"
}
fun collectorWork(c: Collector?): String = when(c) {
is IdpCollector -> "idp"
else -> "error"
}
fun main() {
println(Collector().let { c -> c.isOrDoesWork(c) || collectorWork(c as? Collector) })
}
KT
kotlinc "$tmp/Main.kt" -include-runtime -d "$tmp/out.jar" 2>&1 | head -80 || true
else
echo "kotlinc not available"
fiRepository: ForgeRock/ping-react-native-sdk
Length of output: 13238
Make DaVinci runtime handle IdpCollector safely when external-idp is absent.
DaVinciNodeMapper.mapCollectorPayload is executed for every collector and references com.pingidentity.idp.davinci.IdpCollector directly; with external-idp marked compileOnly, it is not guaranteed to be on the consumer app’s runtime classpath. If an app uses only rn-davinci without rn-external-idp, collector mapping can fail. Guard IdpCollector at runtime or add the dependency so this required native path does not throw.
🤖 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 `@packages/davinci/android/build.gradle` around lines 99 - 100, Make
IdpCollector available at runtime for DaVinciNodeMapper.mapCollectorPayload by
changing the external-idp dependency from compileOnly to the appropriate
runtime-inclusive configuration. Preserve the existing collector mapping
behavior so apps using rn-davinci alone cannot fail with a missing IdpCollector
class.
| let collector = matching[collectorIndex] | ||
| let callbackURLScheme = callConfig.redirectUri.isEmpty ? nil : callConfig.redirectUri | ||
| let result = await collector.authorize(callbackURLScheme: callbackURLScheme) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'callbackURLScheme|redirectUri|authorizeForDaVinci' \
packages/external-idp packages/davinciRepository: ForgeRock/ping-react-native-sdk
Length of output: 50388
🌐 Web query:
For Ping Identity DaVinci iOS SDK version 2.0.1, does IdpCollector.authorize(callbackURLScheme:) require only a URL scheme, or does it accept a complete redirect URI?
💡 Result:
For Ping Identity DaVinci iOS SDK version 2.0.1, the IdpCollector.authorize method does not take a parameter for a redirect URI or scheme [1]. The authentication process is managed by the SDK, which relies on the configuration established in the PingOne platform and your iOS app's Info.plist [2][3]. When integrating external Identity Providers (IdPs) in your iOS application, you must configure a custom URL scheme in your app's Xcode project settings (specifically in the URL Types panel) to handle the callback after authentication [2][3]. The corresponding value (e.g., myapp://example.com) must also be set in the PingOne Form connector's Application Return URL field to ensure the IdP knows where to redirect the user upon completion [3]. The SDK automatically handles the interaction with the external IdP, and you initiate this process simply by calling authorize on the IdpCollector object [1].
Citations:
- 1: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/03_authenticate_with_external_idps.html
- 2: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/02_handling_uri_schemes.html
- 3: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/before-you-begin.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Candidate files:"
fd -a 'RNPingExternalIdpCommon.swift|authenticate_with_external_idps|handling_uri_schemes' . | sed 's#^\./##' | head -50
echo
echo "Relevant ios Swift declarations:"
rg -n -C 4 'func authorize|authorize\(callbackURLScheme|callbackURLScheme:' packages/external-idp/ios/RNPingExternalId pCommon.swift packages 2>/dev/null || trueRepository: ForgeRock/ping-react-native-sdk
Length of output: 12733
🌐 Web query:
site:developer.pingidentity.com OrchSDKs DaVinci iOS authorize callbackURLScheme IdpCollector
💡 Result:
In the Ping Identity Orchestration (DaVinci) SDK for iOS, the IdpCollector is a component used to facilitate social sign-on (authentication with external identity providers like Apple, Google, or Facebook) [1][2]. To implement social sign-on with the IdpCollector, you must integrate the PingExternalIdp library alongside your core DaVinci dependency (such as PingDavinci) [3]. The core components of this process involve: 1. Handling URI Schemes: Because the authentication flow redirects users to an external identity provider and back to your app, you must configure a custom URL scheme in Xcode under your project's Info tab [4]. This ensures your app is correctly brought back to the foreground when the authentication process finishes [4]. 2. Using IdpCollector: When a DaVinci flow reaches an external IdP step, it returns an IdpCollector [1][5]. Your application code should detect this collector (often within a ViewModel), and call the authorize method on the IdpCollector instance to initiate the external authentication flow [1]. Example Swift usage: public func startSocialAuthentication async -> Result<Bool, IdpExceptions> { return await idpCollector.authorize } The authorize method is asynchronous and returns a result indicating whether the authentication succeeded or failed [1]. If successful, the flow typically continues; if not, it returns a failure with associated IdpExceptions [1].
Citations:
- 1: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/03_authenticate_with_external_idps.html
- 2: https://developer.pingidentity.com/orchsdks/davinci/try-it-out/ios/00_before-you-begin.html
- 3: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/01_adding_core_dependencies.html
- 4: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/02_handling_uri_schemes.html
- 5: https://developer.pingidentity.com/orchsdks/davinci/usage/ios/03-stepping-through-davinci-flows.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect relevant Swift implementation and config parser:"
sed -n '230,315p' packages/external-idp/ios/RNPingExternalIdpCommon.swift
echo
sed -n '240,265p' packages/external-idp/ios/Config/ExternalIdpConfigParser.swift
echo
echo "Check for callbackURLScheme parsing/normalization:"
rg -n 'callbackURLScheme|callbackScheme|parsedUrl|URL(string|path|init)|scheme' packages/external-idp/ios packages/browser/ios tests 2>/dev/null || true
echo
echo "List repo packages that define/dependency PingExternalIdP:"
rg -n 'PingExternalIdP|ExternalIdp|IdpCollector\.authorize|IdpCollector' packages -g '*.podspec' -g '*.pod' -g '*.swift' -g '*.kt' -g '*.java' -g '*.mm' | head -200Repository: ForgeRock/ping-react-native-sdk
Length of output: 3623
🌐 Web query:
"PingExternalIdP" "IdpCollector" "authorize(callbackURLScheme:"
💡 Result:
In the Ping Identity SDKs (such as the iOS Unified SDK), the IdpCollector class provides an authorize method used to initiate authentication with an external Identity Provider (IdP) [1][2]. The method is defined as follows in Swift: open func authorize(callbackURLScheme: String? = nil) async -> Result<Bool, IdpExceptions> [1] The callbackURLScheme parameter is an optional string used to specify the custom URI scheme for returning to your application after the external authentication process completes [1]. When calling this method, you provide the scheme configured in your PingOne application settings to ensure the browser or external user-agent can successfully redirect the user back to your app [3]. Key details regarding the authorize method: - Asynchronous execution: The method is asynchronous and returns a Result type [1][2]. - Result Handling: It returns a success (indicating the authentication flow was initiated or completed as expected) or a failure, which includes IdpExceptions detailing any root causes for issues [4][2]. - Usage: It is typically invoked when your application handles an IdpCollector node received from the DaVinci orchestration flow [4][2]. - Android variance: In Android SDKs, the authorize method may require a URI parameter, such as a redirect URI string, to facilitate the callback process [5].
Citations:
- 1: https://developer.pingidentity.com/reference/sdks/ios/unified/api-reference-2-0-0/ExternalIdP/Classes/IdpCollector.html
- 2: https://docs.pingidentity.com/sdks/latest/davinci/use-cases/social-login/ios/03_handling_idpcollector_nodes.html
- 3: https://docs.pingidentity.com/sdks/latest/davinci/use-cases/social-login/ios/index.html
- 4: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/ios/03_authenticate_with_external_idps.html
- 5: https://developer.pingidentity.com/orchsdks/davinci/use-cases/external-idp/android/03_authenticate_with_external_idps.html
Use only the callback scheme for DaVinci external IdP authorization.
IdpCollector.authorize(callbackURLScheme:) expects the custom URL scheme, but callConfig.redirectUri is passed as the full redirect URI such as com.myapp://callback. Extract the scheme before this call so the SDK can route the auth callback back to the app.
🤖 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 `@packages/external-idp/ios/RNPingExternalIdpCommon.swift` around lines 302 -
304, The authorization flow in IdpCollector.authorize must receive only the
callback URL scheme, not the full redirect URI. Update the callbackURLScheme
construction near callConfig.redirectUri to parse and extract its scheme while
preserving nil for an empty redirect URI, then pass that value to authorize.
| <key>GIDServerClientID</key> | ||
| <string>$(GOOGLE_IOS_CLIENT_ID)</string> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.xcconfig' --glob '*.pbxproj' --glob '*.plist' \
'GOOGLE_(IOS|SERVER)_CLIENT_ID' PingSampleApp packagesRepository: ForgeRock/ping-react-native-sdk
Length of output: 469
🌐 Web query:
Google Sign-In for iOS GIDServerClientID backend OAuth client ID developers.google.com
💡 Result:
The GIDServerClientID key is used in your iOS or macOS application's Info.plist file to configure the Google Sign-In SDK for backend authentication or offline access [1][2][3]. When your application needs to communicate with a backend server—such as to verify the user's ID token or exchange an authorization code for tokens—it must identify your backend project to Google's authentication system [4][1]. The GIDServerClientID serves this purpose [1]. Key details regarding GIDServerClientID: 1. Purpose: It represents your backend server's OAuth client ID, not your iOS/macOS application's client ID [1]. Providing this allows the SDK to include the correct audience information in the ID token or to request an authorization code that your specific backend is authorized to exchange [5][4][6]. 2. Configuration: You set this by adding the GIDServerClientID key to your Info.plist file with your web application's OAuth client ID as the string value [1][3]: GIDServerClientID YOUR_SERVER_CLIENT_ID 3. Obtaining the ID: You must create this client ID in the Google Cloud Console by selecting the Web application type for your project's OAuth client ID [1]. If you do not include this key, you will be unable to securely authenticate users on your backend or perform offline access actions like obtaining a serverAuthCode [5][4][1]. For most integrations, this is an optional key used only if your app requires a backend component [1][3].
Citations:
- 1: https://developers.google.com/identity/sign-in/ios/start-integrating
- 2: https://developers.google.com/identity/sign-in/ios/quick-migration-guide
- 3: https://developers.google.com/identity/sign-in/ios/release
- 4: https://developers.google.com/identity/sign-in/ios/backend-auth
- 5: https://developers.google.com/identity/sign-in/ios/offline-access
- 6: https://developers.google.com/identity/sign-in/ios/reference/Classes/GIDConfiguration
Use the backend OAuth client ID for GIDServerClientID.
GIDServerClientID must be the web/server OAuth client ID used by the backend, while GIDClientID should remain the iOS client ID.
Proposed fix
<key>GIDServerClientID</key>
-<string>$(GOOGLE_IOS_CLIENT_ID)</string>
+<string>$(GOOGLE_SERVER_CLIENT_ID)</string>📝 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.
| <key>GIDServerClientID</key> | |
| <string>$(GOOGLE_IOS_CLIENT_ID)</string> | |
| <key>GIDServerClientID</key> | |
| <string>$(GOOGLE_SERVER_CLIENT_ID)</string> |
🤖 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 `@PingSampleApp/ios/PingSampleApp/Info.plist` around lines 66 - 67, Update the
GIDServerClientID entry in Info.plist to reference the backend web/server OAuth
client ID configuration, while preserving GIDClientID as the iOS client ID. Use
the existing backend client-ID symbol or setting rather than
GOOGLE_IOS_CLIENT_ID.
| useEffect(() => { | ||
| if (node?.type !== 'ContinueNode') { | ||
| lastNodeKeyRef.current = null; | ||
| return; | ||
| } | ||
| if (!enabled) return; | ||
| if (loading) return; | ||
| if (!continueNodeKey) return; | ||
| if (lastNodeKeyRef.current === continueNodeKey) return; | ||
|
|
||
| const { meta, canSubmit, input } = form; | ||
| if (meta.hasManual) return; | ||
| if (meta.hasIntegrationRequired) return; | ||
| if (meta.hasUnsupported) return; | ||
| if (!canSubmit) return; | ||
|
|
||
| lastNodeKeyRef.current = continueNodeKey; | ||
| onForwarded(); | ||
| void next(input).catch(() => { | ||
| // Error state is surfaced by the useDaVinci hook. | ||
| }); | ||
| }, [continueNodeKey, enabled, form, loading, next, node?.type, onForwarded]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
idpJustAuthorized can get stuck true, later auto-forwarding unrelated nodes (including a fresh flow's first node).
The flag is only ever cleared by onForwarded() inside the auto-forwarder's success path. If the post-authorize ContinueNode requires manual input, the effect returns early without clearing it, and nothing else (including onStart) resets it either — so it can leak into unrelated later nodes or a subsequent start(), contradicting the hook's own "only immediately after IdP authorize" / "never fires on the initial start() node" invariants.
PingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.ts#L73-L94: clear the window (callonForwarded()) as soon as aContinueNodecheck concludes it can't auto-forward (manual/unsupported/integration-required/!canSubmit), not only on a successful forward.PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts#L122-L131: as a defense-in-depth backstop, also resetsetIdpJustAuthorized(false)inonStartalongsidesetIdpError(null).
📍 Affects 2 files
PingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.ts#L73-L94(this comment)PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts#L122-L131
🤖 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 `@PingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.ts` around lines 73 -
94, Prevent idpJustAuthorized from leaking past the immediate post-authorization
ContinueNode: in
PingSampleApp/ui/davinci/hooks/useDaVinciAutoForwarder.ts:73-94, call
onForwarded() when the ContinueNode cannot auto-forward due to manual input,
integration requirements, unsupported state, or !canSubmit, while preserving the
successful-forward path; in
PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts:122-131, reset
setIdpJustAuthorized(false) in onStart alongside setIdpError(null).
| const onIdpAuthorize = useCallback( | ||
| async (collector: IdpCollector): Promise<void> => { | ||
| if (loading) { | ||
| return; | ||
| } | ||
| const davinciClient = davinciContext?.client; | ||
| if (!davinciClient) { | ||
| console.warn( | ||
| '[DaVinci] authorizeForDaVinci: no DaVinci client in context', | ||
| ); | ||
| return; | ||
| } | ||
| // Determine which index this collector occupies among IDP collectors on the | ||
| // current node so the native side can resolve the right IdpCollector. | ||
| const idpFields = form.fields.filter( | ||
| f => f.type === 'SOCIAL_LOGIN_BUTTON', | ||
| ); | ||
| const index = idpFields.findIndex(f => f.key === collector.key); | ||
| try { | ||
| // authorizeForDaVinci opens the IdP browser flow and sets the | ||
| // resume request internally — token flows through the subsequent next(). | ||
| await externalIdp.authorizeForDaVinci(davinciClient, { | ||
| index: index >= 0 ? index : 0, | ||
| }); | ||
| setIdpJustAuthorized(true); | ||
| await next({ collectors: [] }); | ||
| } catch (err) { | ||
| console.warn('[DaVinci] authorizeForDaVinci failed:', err); | ||
| const msg = err instanceof Error ? err.message : String(err); | ||
| setIdpError(msg); | ||
| } | ||
| }, | ||
| [davinciContext?.client, externalIdp, form.fields, loading, next], | ||
| ); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fallback to index 0 on unmatched collector could authorize the wrong IdP.
If collector.key isn't found among SOCIAL_LOGIN_BUTTON fields, index >= 0 ? index : 0 silently defaults to the first IdP collector rather than surfacing an error — when there are multiple social-login buttons, this could authorize a different provider than the one the user tapped.
🐛 Suggested fix
const index = idpFields.findIndex(f => f.key === collector.key);
+ if (index < 0) {
+ console.warn('[DaVinci] authorizeForDaVinci: collector key not found among SOCIAL_LOGIN_BUTTON fields');
+ setIdpError('Unable to resolve the selected sign-in provider.');
+ return;
+ }
try {
await externalIdp.authorizeForDaVinci(davinciClient, {
- index: index >= 0 ? index : 0,
+ index,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onIdpAuthorize = useCallback( | |
| async (collector: IdpCollector): Promise<void> => { | |
| if (loading) { | |
| return; | |
| } | |
| const davinciClient = davinciContext?.client; | |
| if (!davinciClient) { | |
| console.warn( | |
| '[DaVinci] authorizeForDaVinci: no DaVinci client in context', | |
| ); | |
| return; | |
| } | |
| // Determine which index this collector occupies among IDP collectors on the | |
| // current node so the native side can resolve the right IdpCollector. | |
| const idpFields = form.fields.filter( | |
| f => f.type === 'SOCIAL_LOGIN_BUTTON', | |
| ); | |
| const index = idpFields.findIndex(f => f.key === collector.key); | |
| try { | |
| // authorizeForDaVinci opens the IdP browser flow and sets the | |
| // resume request internally — token flows through the subsequent next(). | |
| await externalIdp.authorizeForDaVinci(davinciClient, { | |
| index: index >= 0 ? index : 0, | |
| }); | |
| setIdpJustAuthorized(true); | |
| await next({ collectors: [] }); | |
| } catch (err) { | |
| console.warn('[DaVinci] authorizeForDaVinci failed:', err); | |
| const msg = err instanceof Error ? err.message : String(err); | |
| setIdpError(msg); | |
| } | |
| }, | |
| [davinciContext?.client, externalIdp, form.fields, loading, next], | |
| ); | |
| const onIdpAuthorize = useCallback( | |
| async (collector: IdpCollector): Promise<void> => { | |
| if (loading) { | |
| return; | |
| } | |
| const davinciClient = davinciContext?.client; | |
| if (!davinciClient) { | |
| console.warn( | |
| '[DaVinci] authorizeForDaVinci: no DaVinci client in context', | |
| ); | |
| return; | |
| } | |
| // Determine which index this collector occupies among IDP collectors on the | |
| // current node so the native side can resolve the right IdpCollector. | |
| const idpFields = form.fields.filter( | |
| f => f.type === 'SOCIAL_LOGIN_BUTTON', | |
| ); | |
| const index = idpFields.findIndex(f => f.key === collector.key); | |
| if (index < 0) { | |
| console.warn( | |
| '[DaVinci] authorizeForDaVinci: collector key not found among SOCIAL_LOGIN_BUTTON fields', | |
| ); | |
| setIdpError('Unable to resolve the selected sign-in provider.'); | |
| return; | |
| } | |
| try { | |
| // authorizeForDaVinci opens the IdP browser flow and sets the | |
| // resume request internally — token flows through the subsequent next(). | |
| await externalIdp.authorizeForDaVinci(davinciClient, { | |
| index, | |
| }); | |
| setIdpJustAuthorized(true); | |
| await next({ collectors: [] }); | |
| } catch (err) { | |
| console.warn('[DaVinci] authorizeForDaVinci failed:', err); | |
| const msg = err instanceof Error ? err.message : String(err); | |
| setIdpError(msg); | |
| } | |
| }, | |
| [davinciContext?.client, externalIdp, form.fields, loading, next], | |
| ); |
🤖 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 `@PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts` around
lines 169 - 203, Update onIdpAuthorize to handle a missing collector match
explicitly instead of passing index 0 to authorizeForDaVinci. When findIndex
returns -1, surface an appropriate error through the existing IdP error handling
and return without authorizing; preserve the current authorization flow for
valid indices.
1 participant
Summary
IdpCollector(SOCIAL_LOGIN_BUTTON) support to the DaVinci bridge on iOS and Android'IDP'normalization to the server-native'SOCIAL_LOGIN_BUTTON'string, matching the pattern used by all other collectorsrawFieldKeyhelper for collectors whoseid()doesn't match the serverkeyfield (extensible for future collectors)setDaVinciCollectorResolverfrom eagerinitintoconfigureDaVincion iOS, matching the Journey patternintegrationRequiredCollectorTypestoSet<DaVinciCollector['type']>for compile-time safetyDaVinciIdpFieldcomponent anduseDaVinciAutoForwarderhook to the sample app for seamless post-IdP-authorize auto-advanceUserProfileInfoCardTest plan
yarn test --filter=@ping-identity/rn-davinci --filter=@ping-identity/rn-external-idpyarn typecheckSummary by CodeRabbit
New Features
Bug Fixes