feat(davinci): add SOCIAL_LOGIN_BUTTON IdP collector support (SDKS-5128) - #59
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 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 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (34.21%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. 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.
There was a problem hiding this comment.
Not a cycle — this is the native SDK artifact com.pingidentity.sdks:davinci, not the rn-davinci RN module.
There was a problem hiding this comment.
@pingidentity-gaurav, thanks for clarifying — agreed. com.pingidentity.sdks:davinci is the native SDK artifact, not the rn-davinci React Native module, so making native external-idp runtime-inclusive does not create that module cycle.
The finding remains: since DaVinciNodeMapper directly references IdpCollector, compileOnly can leave that class unavailable at runtime for consumers using rn-davinci alone. The dependency should therefore use implementation rather than compileOnly.
✏️ Learnings added
Learnt from: pingidentity-gaurav
Repo: ForgeRock/ping-react-native-sdk PR: 59
File: packages/davinci/android/build.gradle:99-100
Timestamp: 2026-07-28T20:14:48.388Z
Learning: In `packages/davinci/android/build.gradle`, `com.pingidentity.sdks:davinci` is the native Android SDK artifact and is distinct from the `rn-davinci` React Native module; adding the native `com.pingidentity.sdks:external-idp` artifact as an implementation dependency does not create an RN-module dependency cycle.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| 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.
There was a problem hiding this comment.
We pass nil when no redirectUri is provided, which is correct — the native SDK picks the registered URL scheme from Info.plist.
There was a problem hiding this comment.
@pingidentity-gaurav, agreed—passing nil when redirectUri is absent is correct and should retain the SDK’s Info.plist-based scheme selection.
The finding is limited to the non-empty case: if redirectUri can be a complete URI (for example, com.myapp://callback), it should be reduced to com.myapp before passing it as callbackURLScheme. If the API contract guarantees redirectUri is already scheme-only, please let me know and I’ll withdraw the comment.
| 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).
There was a problem hiding this comment.
onForwarded clears the flag immediately when the forwarder fires. Fresh flows remount the component, resetting state. Not a real issue.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts --items all --type function --view expanded
ast-grep outline PingSampleApp/ui/davinci/components/organisms/DaVinciClientPanel.tsx --items all --type function --view expanded
printf '\n--- controller state/start/forwarder context ---\n'
rg -n -C 8 'idpJustAuthorized|setIdpJustAuthorized|onStart|useDaVinciAutoForwarder' \
PingSampleApp/ui/davinci/hooks/useDaVinciClientPanelController.ts
printf '\n--- controller ownership context ---\n'
rg -n -C 6 'useDaVinciClientPanelController|DaVinciClientPanel' \
PingSampleApp/ui/davinci/components/organisms/DaVinciClientPanel.tsx \
PingSampleApp/ui/DaVinciScreen.tsxLength of output: 12965
@pingidentity-gaurav, thanks for clarifying. I rechecked the current control flow, and the concern still applies to the non-forwarding path:
onForwarded()does clearidpJustAuthorizedimmediately—but it is called only after the node passes every auto-forward eligibility check.- If the post-IdP
ContinueNodehas manual input, integration-required/unsupported fields, or cannot submit, the hook returns without clearing the flag. - The panel controller remains mounted while navigating within this flow, and
onStart()currently does not reset the state either.
So remounting protects a later fresh screen instance, but not an in-place continuation after an ineligible post-authorization node. The reset should remain on those early-return paths (with the onStart reset as a backstop).
|
PR reviewed. Great work! Left minor comments. |
rodrigoareis
left a comment
rodrigoareis
left a comment
There was a problem hiding this comment.
Changes looks good to me. Just a minor comment on the sample app.
…etry, add test_spec deps (SDKS-5128)
3 participants
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