Skip to content

SDK-2746: .NET - Add Enforce Handoff to SDKs for IDV Create Session - dotnet#544

Open
mehmet-yoti wants to merge 1 commit into
masterfrom
websdk-auto/SDK-2746-dotnet-net---add-enforce-handoff-to-sdks-for-idv-create-session
Open

SDK-2746: .NET - Add Enforce Handoff to SDKs for IDV Create Session - dotnet#544
mehmet-yoti wants to merge 1 commit into
masterfrom
websdk-auto/SDK-2746-dotnet-net---add-enforce-handoff-to-sdks-for-idv-create-session

Conversation

@mehmet-yoti
Copy link
Copy Markdown
Contributor

Summary

Adds support for the enforce_handoff flag to the IDV Create Session SDK configuration. When enabled, the user is required to perform mobile handoff to upload their resources. The flag is wired through SdkConfig and exposed via a new WithEnforceHandoff(bool) method on SdkConfigBuilder, with full unit-test coverage.

Changes

  • src/Yoti.Auth/DocScan/Session/Create/SdkConfig.cs
    • Added nullable EnforceHandoff property serialized as enforce_handoff.
    • Extended the constructor with a new optional enforceHandoff parameter (defaults to null for backwards compatibility).
  • src/Yoti.Auth/DocScan/Session/Create/SdkConfigBuilder.cs
    • Added _enforceHandoff backing field and a new fluent WithEnforceHandoff(bool) builder method, with XML docs noting the server-side rule that enforce_handoff cannot be true while allow_handoff is false (DOCS-3523).
    • Updated Build() to pass the new value into the SdkConfig constructor.
  • test/Yoti.Auth.Tests/DocScan/Session/Create/SdkConfigBuilderTests.cs
    • Added tests covering: building with EnforceHandoff = true, false, default null, JSON omission when unset, and combined use with WithAllowHandoff.

QA Test Steps

  1. Setup
    • Check out the branch websdk-auto/SDK-2746-dotnet-net---add-enforce-handoff-to-sdks-for-idv-create-session.
    • Restore and build the solution: dotnet restore && dotnet build.
  2. Run unit tests
    • Execute dotnet test and confirm all tests pass, in particular the new tests in SdkConfigBuilderTests.cs:
      • ShouldBuildWithEnforceHandoffTrue
      • ShouldBuildWithEnforceHandoffFalse
      • EnforceHandoffShouldBeNullIfNotSet
      • EnforceHandoffShouldBeOmittedFromJsonWhenNotSet
      • ShouldBuildWithBothAllowHandoffAndEnforceHandoff
  3. Happy path — JSON serialization
    • In a sample/test project, build a session payload using:
      var sdkConfig = new SdkConfigBuilder()
          .WithAllowHandoff(true)
          .WithEnforceHandoff(true)
          .Build();
    • Serialize to JSON and confirm both "allow_handoff":true and "enforce_handoff":true appear in the payload.
  4. Edge case — flag omitted
    • Build an SdkConfig without calling WithEnforceHandoff(...), serialize using NullValueHandling.Ignore, and confirm the JSON does NOT contain the enforce_handoff key.
  5. Edge case — explicit false
    • Build with .WithEnforceHandoff(false).Build() and confirm the JSON contains "enforce_handoff":false.
  6. Server-side validation
    • Submit an IDV Create Session request via the IDV API with allow_handoff=false and enforce_handoff=true and confirm the API rejects it (validation is server-side; the SDK does not pre-validate). See DOCS-3523.
    • Submit a valid combination (allow_handoff=true, enforce_handoff=true) and confirm the session is created and the user is forced into the mobile handoff flow.
  7. Regression checks
    • Existing WithAllowHandoff tests (ShouldBuildWithMobileHandoffTrue/False, MobileHandoffShouldBeNullIfNotSet) still pass.
    • Existing SdkConfig consumers compile without changes (the new constructor parameter is optional and defaulted).
    • Sessions created without invoking WithEnforceHandoff produce identical JSON to previous versions of the SDK.

Notes

  • The new constructor parameter enforceHandoff is appended after idDocumentTextDataExtractionRetriesConfig as an optional argument to preserve binary/source compatibility for existing callers.
  • No client-side validation is performed for the allow_handoff=false + enforce_handoff=true invalid combination; this is enforced by the IDV API server-side per DOCS-3523, matching the approach used by the other Yoti SDKs.
  • enforce_handoff is serialized only when explicitly set, ensuring older backends that do not recognize the field continue to receive unchanged payloads when the feature is not used.

Related Jira: SDK-2746
Auto-generated by n8n + Claude CLI

@mehmet-yoti
Copy link
Copy Markdown
Contributor Author

🤖 Claude Code Review

Code Review Findings

Summary

The branch adds an optional enforce_handoff boolean to the IDV SdkConfig/SdkConfigBuilder, mirroring the existing allow_handoff pattern. The implementation is small, follows established conventions (nullable bool, JSON snake_case property, fluent WithX builder method), and is covered by tests including JSON serialization assertions. A few real issues are worth addressing before merge — most importantly an API/binary breaking change in the SdkConfig constructor signature, plus some minor polish items.


Critical

None.


Major

1. Breaking change to public SdkConfig constructor signature — enforceHandoff is appended after idDocumentTextDataExtractionRetriesConfig

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfig.cs:49-60
  • Issue: SdkConfig is a public class with a public constructor. Although the new enforceHandoff parameter has a default value, adding a parameter after an optional parameter is a binary-breaking change. Any consumer that compiled against the previous SDK and uses the constructor directly (positional or named) will need to recompile; positional calls that supplied idDocumentTextDataExtractionRetriesConfig continue to compile but external libraries that took the previous compiled call site will fail at runtime with a MissingMethodException. This is meaningful because the DocScan SDK is published as a NuGet package consumed by third parties.
  • Why it matters: Even default-value parameter additions break ABI in .NET because optional parameters are baked into the call site at compile time. This could cause production surprises after a NuGet upgrade.
  • Recommendation: Either (a) add an overload that delegates to a private full-parameter constructor (preferred), or (b) at minimum document this as a breaking change in the release notes. Example:
    public SdkConfig(string allowedCaptureMethods, ..., bool? allowHandoff = null,
                     Dictionary<string, int> idDocumentTextDataExtractionRetriesConfig = null)
        : this(allowedCaptureMethods, ..., allowHandoff, idDocumentTextDataExtractionRetriesConfig, enforceHandoff: null) { }
    
    public SdkConfig(string allowedCaptureMethods, ..., bool? allowHandoff,
                     Dictionary<string, int> idDocumentTextDataExtractionRetriesConfig,
                     bool? enforceHandoff) { ... }

2. Constructor parameter ordering breaks the natural grouping

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfig.cs:58-60
  • Issue: enforceHandoff is placed after idDocumentTextDataExtractionRetriesConfig, separating it from the closely-related allowHandoff parameter. The two booleans should logically sit next to each other (and they would, in any new internal overload), and the property-declaration order at lines 38–44 already groups them together. The mismatch between property order and constructor order is a small footgun for future maintainers.
  • Recommendation: If you introduce the overload pattern recommended above, place enforceHandoff immediately after allowHandoff in the new full-parameter constructor. The legacy public constructor can keep the old shape for compatibility.

Minor

3. Server-side-only validation leaves the SDK silently producing invalid configs

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfigBuilder.cs:151-175
  • Issue: A caller can do .WithAllowHandoff(false).WithEnforceHandoff(true) and the SDK will happily serialize that combination, only failing at the IDV API. The XML doc and the comment in SdkConfig.cs:41-42 acknowledge this constraint — relying on server-side validation is a deliberate decision (DOCS-3523) — but client-side guard rails would shorten the dev feedback loop and avoid a network round-trip to discover a static configuration error.
  • Recommendation: Consider validating in Build() (throw InvalidOperationException when _allowHandoff == false && _enforceHandoff == true). If the team prefers to keep validation server-side for consistency with other SDKs, that's a defensible choice — just confirm it's intentional across SDKs.

4. Trailing newline removed at end of SdkConfigBuilder.cs

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfigBuilder.cs:281
  • Issue: The diff ends with \ No newline at end of file. Other source files in this repo (e.g., DocScanService.cs:363) end with a newline, and POSIX text files conventionally do.
  • Recommendation: Add a trailing newline.

5. Negative-assertion test uses Assert.IsFalse(string.Contains(...)) instead of a clearer message form

  • File: test/Yoti.Auth.Tests/DocScan/Session/Create/SdkConfigBuilderTests.cs:229
  • Issue: Assert.IsFalse(json.Contains("enforce_handoff")) produces an unhelpful failure message ("Assert.IsFalse failed") that doesn't show the offending JSON. Other assertions in the file use StringAssert.Contains, which prints the actual string on failure.
  • Recommendation: Use Assert.IsFalse(json.Contains("enforce_handoff"), $"Expected JSON to omit enforce_handoff but got: {json}"); or StringAssert.DoesNotMatch(json, new Regex("enforce_handoff"));.

6. Missing test: WithEnforceHandoff should retain latest value when called multiple times

  • File: test/Yoti.Auth.Tests/DocScan/Session/Create/SdkConfigBuilderTests.cs
  • Issue: ShouldRetainLatestAllowedMethod (lines 35–44) covers idempotence for capture methods, but neither WithAllowHandoff nor the new WithEnforceHandoff has a "last call wins" test. Worth adding for parity, since builder semantics are part of the contract.
  • Recommendation: Add a test calling WithEnforceHandoff(true).WithEnforceHandoff(false) and assert the final value is false.

7. Inconsistent fluent-return assertion: Assert.AreSame(builder, returned) only on WithEnforceHandoff(true)

  • File: test/Yoti.Auth.Tests/DocScan/Session/Create/SdkConfigBuilderTests.cs:181
  • Issue: Inconsistent — only the new test asserts the builder returns this. Either drop the assertion (it's low-value given the type system already enforces a SdkConfigBuilder return) or apply it consistently.
  • Recommendation: Drop Assert.AreSame(builder, returned) from ShouldBuildWithEnforceHandoffTrue for consistency with the rest of the file, or add a dedicated minimal "returns same instance" test if that coverage is desired.

Nit

8. XML doc wording could match the existing WithAllowHandoff style more closely

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfigBuilder.cs:157-170
  • Issue: WithAllowHandoff mentions the precedence story ("Passing this value will override any value set in the Yoti Connect backend…"). The new WithEnforceHandoff doc doesn't, and it's reasonable to assume the same precedence rules apply.
  • Recommendation: If precedence behaves the same way, mirror the wording so users don't have to guess.

9. Inline source comment duplicates the XML doc remark

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfig.cs:41-42
  • Issue: The two-line // comment above the property restates what the builder's XML <remarks> already says. The XML doc is the discoverable surface (IntelliSense); the inline comment will quietly drift.
  • Recommendation: Either remove the inline comment or convert it into an XML <summary>/<remarks> on the property itself for IntelliSense visibility.

10. JSON property attribute style is consistent with file conventions

  • File: src/Yoti.Auth/DocScan/Session/Create/SdkConfig.cs:43
  • Issue: This file already uses the verbose [JsonProperty(PropertyName = "...")] form everywhere, so the new attribute matches surrounding code — no action required. Noted only to confirm the chosen form is intentionally consistent with the file's existing style.
  • Recommendation: No change.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for an enforce_handoff flag in the Doc Scan IDV Create Session SDK configuration so callers can require mobile handoff when uploading resources.

Changes:

  • Added nullable EnforceHandoff to SdkConfig, serialized as enforce_handoff.
  • Extended SdkConfigBuilder with _enforceHandoff and a fluent WithEnforceHandoff(bool) method.
  • Added/updated unit tests to cover EnforceHandoff behavior and JSON serialization/omission when unset.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Yoti.Auth/DocScan/Session/Create/SdkConfig.cs Adds EnforceHandoff property and updates the public constructor to accept it.
src/Yoti.Auth/DocScan/Session/Create/SdkConfigBuilder.cs Adds backing field + builder method and wires value into SdkConfig construction.
test/Yoti.Auth.Tests/DocScan/Session/Create/SdkConfigBuilderTests.cs Adds tests verifying EnforceHandoff values and JSON serialization behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +241 to +242
Assert.IsTrue(sdkConfig.AllowHandoff);
Assert.IsTrue(sdkConfig.EnforceHandoff);
@@ -51,7 +56,8 @@ public SdkConfig(string allowedCaptureMethods,
string errorUrl,
string privacyPolicyUrl,
bool? allowHandoff = null,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants