feat(android): opt-in signing-URL launch strategy for presentCaptiveSigning - #4
Draft
IronTony wants to merge 2 commits into
Draft
feat(android): opt-in signing-URL launch strategy for presentCaptiveSigning#4IronTony wants to merge 2 commits into
IronTony wants to merge 2 commits into
Conversation
…ning The SDK's fetch-based launchCaptiveSigning downloads the envelope with include=documents before opening the ceremony. That download runs on a read timeout derived from the envelope size which floors at 15s when nothing is cached, so a large envelope on a slow connection exhausts it and the ceremony never opens. The SDK also exposes a signingURL overload that skips the download. Minting a recipient view needs only the recipient details already passed to presentCaptiveSigning plus the session credentials, so opting into launchStrategy 'signingUrl' takes the timing-out call out of the code path. Opt-in, defaulting to 'fetch', because the mint spends the consumer's bearer token on an endpoint that token is not guaranteed to be scoped for. A mint failure falls back to the fetch path, so the strategy can only add a way to succeed. Verified on device across 12 consecutive ceremonies. Two hazards the async hop introduces, both guarded: Minting puts a network round trip between capturing the Activity and using it, which the fetch path never did. canLaunchOn refuses to launch against a finishing Activity, and checks the pending completion by identity rather than nullness. reset() and endSigningSession() clear that slot and a fresh call can claim it while a mint is still in flight; a nullness check would pass in that window and launch this envelope wired to the new session's promise. Session credentials move into a single DocuSignSession reference so a login racing an in-flight mint cannot tear the triple and build a request with one session's token and another's account id.
Four cases on the hook: signingUrl and an explicit fetch both reach the native call unchanged, the url flow never carries the field, and omitting it produces a payload with no launchStrategy key at all rather than one set to undefined. That last one is the backward-compatibility guarantee under test. The hook spreads the field in conditionally so an existing consumer who never opts in sends the exact payload previous releases sent, and the test asserts the key is absent rather than merely undefined. README gains an Android launch strategies section covering the timeout this addresses, the token scope the mint needs, and a pointer to presentCaptiveSigningWithUrl as the better shape when the backend can mint.
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
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.
Draft. Depends on #3 and rebases once that merges.
What this adds
An opt-in
launchStrategy: 'signingUrl'onpresentCaptiveSigningfor Android.The SDK's fetch-based
launchCaptiveSigningdownloads the envelope withinclude=documentsbefore opening the ceremony. That download runs on a read timeout derived from the envelope size, which floors at 15s when nothing is cached, so a large envelope on a slow connection exhausts it and the ceremony never opens.The SDK also exposes a
signingURLoverload that skips the download entirely.'signingUrl'mints a recipient view with the session access token and points the SDK straight at it, so the call that times out never runs. Verified on device across 12 consecutive ceremonies.Defaults to
'fetch', which is byte-identical to current behaviour, so upgrading changes nothing for existing consumers. Opt-in rather than default because the mint spends the consumer's bearer token on an endpoint that token is not guaranteed to be scoped for. A mint failure falls back to'fetch', so the strategy can only add a way to succeed.If your backend already mints recipient views,
presentCaptiveSigningWithUrlremains the better shape since it keeps the token off the device entirely. This exists for teams who cannot change their backend.Concurrency notes
Minting puts a network round trip between capturing the Activity and using it, which the synchronous fetch path never did. Two guards:
canLaunchOnrefuses to launch against a finishing or destroyed Activity, and checks the pending completion by identity, not nullness.reset()andendSigningSession()clear that slot, and a freshpresentCaptiveSigningcan claim it while a mint is still in flight. A nullness check would pass in that window and launch this envelope wired to the new session's promise, resolving it with the wrong outcome.DocuSignSessionreference rather than three independent@Volatilefields, so a login racing an in-flight mint cannot tear the triple and build a request with one session's token and another's account id.Known trade-off: the mint thread holds the
Activityreference for the round trip. Timeouts are set to 15s (rather than the 30s this was first written with) to bound that, andcanLaunchOnprevents any use of a dead Activity, but the reference is retained for the duration regardless.After #3 merges
This PR also picks up the maintainer items I pulled off that review. All of them touch pre-existing code rather than anything nikonhub introduced, so they were never that PR's to carry:
presentCaptiveSigningWithUrlinto the shared launcher and extractcaptiveSigningListener(), so the two entrypoints stop carrying duplicate listenerscurrentEnvelopeIdinpresentCaptiveSigning's catch, matching the shape feat: add Android support for captive signing with URL #3 got rightemitSigningErrorin both flows, sorecipient_signing_failedstops flattening intosigning_failedCodedException.codeinstead of hard-codingsigning_failedThrowsbullet once the error code settlesTesting
npm test: 17 passing,useDocuSignSigning.tsat 100% lines / 87% branches. Four new cases cover the passthrough, including that omittinglaunchStrategyproduces a payload with no such key at all rather than one set toundefined, which is the backward-compat guarantee.