[PM-35796] Use SDK bindings for invite link generation

[BTreston]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-35796

📔 Objective

Wires up and unit tests the SDK implementation of generateOrganizationInviteCryptoBundle in OrganizationInviteLinkService via the newly generated WASM bindings from bitwarden/sdk-internal#1157

📸 Screenshots

[BTreston]

@claude review

[claude]

Claude finished @BTreston's task in 2m 36s —— View job

---

Claude PR Review

• Read PR diff and changed files
• Analyze the SDK binding migration
• Verify tests and patterns
• Post review feedback

---

Nice, focused migration from in-repo crypto (KeyGenerationService + EncryptService.wrapSymmetricKey) to the SDK bindings generate_organization_invite_crypto_bundle / unseal_organization_invite_key. The DI cleanup, SdkLoadService.Ready gating, and test-side mocks all look correct. A couple of small regressions and a typing smell are worth tightening before merge.

🐛 Lost null-safety in OrganizationInviteLinkCreateRequest

libs/organization-invite-link/src/models/requests/organization-invite-link-create.request.ts:16

if (!c.encryptedInviteKey.encryptedString) {
  throw new Error("EncryptedInviteKey is required.");
}

The previous check used ?. (!c.encryptedInviteKey?.encryptedString). If a caller passes an undefined encryptedInviteKey, this now throws a generic TypeError: Cannot read properties of undefined (reading 'encryptedString') instead of the intended descriptive error. Restore the optional chain.

🐛 Weakened validation in OrganizationInviteLinkRefreshRequest

libs/organization-invite-link/src/models/requests/organization-invite-link-refresh.request.ts:8

if (!c.encryptedInviteKey) {
  throw new Error("EncryptedInviteKey is required.");
}

Previously this also required encryptedString to be non-empty (!c.encryptedInviteKey?.encryptedString). The new check lets an EncString whose encryptedString is undefined/"" through — which then serializes to a broken wire payload (since EncString.toJSON() returns encryptedString as string). Recommend keeping the stricter !c.encryptedInviteKey?.encryptedString check, matching the create request.

🧹 Typing smell: wrapping a sealed envelope in EncString

libs/organization-invite-link/src/services/default-organization-invite-link.service.ts:148

return new EncString(bundle.sealedInviteKeyEnvelope);

sealedInviteKeyEnvelope is not an EncString-formatted string (<type>.<iv>|<data>|<mac>); it's an opaque sealed-envelope blob. Wrapping it works only because EncString.toJSON() happens to return the raw underlying string — but the resulting object has no usable encryptionType/iv/data/mac fields and any code that introspects it will silently get nothing.

Cleaner options:

• Keep the request DTO field as string (the wire type) and pass bundle.sealedInviteKeyEnvelope directly.
• Or introduce a dedicated SealedInviteKeyEnvelope type so the contract is honest.

This also matches OrganizationInviteLinkResponseModel.encryptedInviteKey: string (responses/organization-invite-link.response.ts:10), so the request side currently disagrees with the response side on the same conceptual field.

🧪 Minor test polish

• default-organization-invite-link.service.spec.ts:170 — description "generates key, wraps with orgKey, calls API, and caches result" still reflects the old implementation. Consider "generates sealed envelope via SDK, calls API, and caches result".
• default-organization-invite-link.service.spec.ts:195, 201 — .rejects.toThrow() (no message) means a regression where the error becomes a TypeError (see first finding) would still pass. Asserting on the message (e.g. "EncryptedInviteKey is required.", "Organization key not found") would have caught the null-safety regression above.

💭 Question

generate_organization_invite_crypto_bundle returns { inviteKey, sealedInviteKeyEnvelope } and the service discards inviteKey, relying on reconstructUrl to re-unseal later. That works, but is the re-unseal-on-every-render trade-off (vs. caching the raw inviteKey in memory after creation) intentional? If yes, no change needed — flagging in case it wasn't deliberate.

---

• Branch: ac/pm-35796-generate-invite-link-crypto

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR migrates OrganizationInviteLinkService from in-repo crypto primitives (KeyGenerationService + EncryptService.wrapSymmetricKey/unwrapSymmetricKey) to the newly available SDK bindings generate_organization_invite_crypto_bundle and unseal_organization_invite_key, with corresponding type changes propagated through request/response models. The change aligns with the repository rule that new encryption logic must not be added to this repo and correctly preserves the zero-knowledge invariant — the raw invite key is generated and sealed inside the SDK and never leaves the client unencrypted.

SdkLoadService.Ready is awaited before invoking the SDK functions, matching the pattern used elsewhere in the codebase. DI registration in jslib-services.module.ts is updated to drop the now-unused EncryptService and KeyGenerationService dependencies. Unit tests are migrated to mock the SDK module and cover create, refresh, and reconstruct paths with appropriate error cases.

Previous reviewer findings from earlier iterations were addressed in commits d23a3c7 and 093a15a. No new actionable findings were identified in this re-review.

Code Review Details

No new findings.

[BTreston]

@claude re-review

[claude]

Claude finished @BTreston's task in 1m 19s —— View job

---

Claude PR Re-Review

• Read current state of changed files
• Compare against previous review feedback
• Post re-review

---

LGTM — the follow-up commit (d23a3c7 update types) addressed every concern from my prior review. Re-reading the final state:

✅ Resolved from previous round

• Null-safety / weaker validation in request DTOs: Both OrganizationInviteLinkCreateRequest and OrganizationInviteLinkRefreshRequest now accept encryptedInviteKey: string directly and validate symmetrically with !c.encryptedInviteKey. The asymmetry between create vs. refresh is gone (organization-invite-link-create.request.ts:14, organization-invite-link-refresh.request.ts:6).
• Typing smell — wrapping a sealed envelope in EncString: generateEncryptedKey now returns the raw bundle.sealedInviteKeyEnvelope string and the DTOs accept string (default-organization-invite-link.service.ts:143-148). The request side now matches OrganizationInviteLinkResponseModel.encryptedInviteKey: string — contract is honest.
• Test polish:
  • The createInviteLink test description is updated to "generates sealed envelope via SDK, calls API, and caches result" (spec.ts:170).
  • Error-message assertions are now specific ("At least one allowed domain is required.", `Organization key not found for org ${mockOrgId}`), so a regression to a TypeError would be caught (spec.ts:195-205, 262-264).

📝 Note on the still-open inline thread

The inline comment on organization-invite-link-refresh.request.ts:6 flagging "Validation for encryptedInviteKey is weaker than the sibling create request" is now stale. After the latest commit, both create and refresh request constructors take encryptedInviteKey: string and use the identical check (!c.encryptedInviteKey). The two are symmetric again — no divergence remains. Suggest resolving that thread.

🔍 Remaining items

None blocking. The DI graph trim in jslib-services.module.ts:1326 (removing EncryptService + KeyGenerationService deps) is consistent with the constructor change, and the spec correctly mocks @bitwarden/sdk-internal + SdkLoadService.Ready (spec.ts:1-11).

The earlier open question about caching the raw inviteKey in memory vs. re-unsealing on each render still stands as a non-blocker — flagging only if it wasn't intentional.

---

✅ Looks good to merge once the stale inline thread is resolved.
• Branch: ac/pm-35796-generate-invite-link-crypto

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud