Skip to content

[PM-41211] Move remaining invite link logic to sdk - #1461

Draft
eliykat wants to merge 6 commits into
mainfrom
ac/pm-41211/ac-clients-sdk-move-remaining-invite-link-logic-to-sdk
Draft

[PM-41211] Move remaining invite link logic to sdk#1461
eliykat wants to merge 6 commits into
mainfrom
ac/pm-41211/ac-clients-sdk-move-remaining-invite-link-logic-to-sdk

Conversation

@eliykat

@eliykat eliykat commented Sep 8, 2026

Copy link
Copy Markdown
Member

🎟️ Tracking

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

📔 Objective

The invite link feature is currently half-in half-out of the SDK. Consolidate all invite link logic in the SDK.

New functions moved to rust:

  • admin CRUD operations: get, update allowed domains, delete
  • user (invitee) operations: get status, validate email

All new functions, and new wrappers around existing functions (create and refresh), now return an OrganizationInviteLinkView. This handles the decryption of the invite secret and the construction of the URL all within the SDK. The client then just receives the constructed URL without having to worry about individual fields like Code or secret. @quexten one nice consequence of this is that the Invite struct won't need to pass the FFI boundary.

One downside is that (as far as I can tell) the SDK doesn't have any environmentService equivalent to get the web vault URL to then construct the full invite link URL. The best it can do is pass a fragment out which TS can then append to the web vault URL. I still think this is an improvement.

As this resulted in the InviteLinkClient becoming quite long, it's been split into 2 sub-clients, following the pattern used by Vault. There is an admin-facing client for admin CRUD operations, and a user-facing client for accepting the link.

🚨 Breaking Changes

No breaking changes expected: there are shims for all functions currently used by TS. These will be deleted after TS has been changed over.

@eliykat eliykat added the ai-review Request a Claude code review label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR splits InviteLinkClient into InviteLinkAdminClient and InviteLinkUserClient, moves the remaining invite link CRUD and invitee operations into the SDK, and introduces OrganizationInviteLinkView so the URL is constructed inside the SDK rather than by TS. I traced the moved code against main and it is a faithful relocation — the account-recovery thumbprint verification, the KeyStoreContext-not-held-across-await scoping in accept_and_optionally_confirm, and the "invite secret is never posted to the server" property are all preserved unchanged. Backwards compatibility looks sound: every method the WASM integration tests still call (create_invite_link, refresh_invite_link, get_invite_secret, accept_and_optionally_confirm) remains on the parent client as a deprecated redirect, and the Android/TypeScript breaking-change checks pass. The only manifest change is enabling the already-vendored workspace http crate for this crate so get can match on StatusCode::NOT_FOUND; Cargo.lock gains no new package.

Code Review Details

No findings met the confidence threshold for posting.

Items considered and dropped during validation:

  • OrganizationInviteLinkView derives Debug while url_fragment embeds the plaintext invite secret. Dropped: decrypted *View types across this codebase (e.g. LoginView, which carries a plaintext password) derive Debug the same way, so this matches an established convention rather than deviating from one.
  • admin().create/refresh delegate to the #[deprecated] create_invite_link/refresh_invite_link, which the description says will be deleted once TS migrates. Dropped: the coupling is deliberate and documented in an inline comment, and the compiler will surface it at deletion time.
  • admin().get maps any 404 to Ok(None). Dropped: this is the conventional "resource absent" mapping and matches the Option return type.
  • No unit tests were added for admin().get, delete, update_allowed_domains, or the NoAllowedDomains guard. Dropped as a generic coverage concern; the crypto path those methods share (to_view) is exercised by create_builds_url_fragment_from_org_code_and_secret.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🔍 SDK Breaking Change Detection

SDK Version: ac/pm-41211/ac-clients-sdk-move-remaining-invite-link-logic-to-sdk (fbc9e98)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client Status Details
typescript ✅ No breaking changes detected Compilation passed with new SDK version - View Details
android ✅ No breaking changes detected Compilation passed with new SDK version - View Details

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.63431% with 145 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.08%. Comparing base (07f075e) to head (fbc9e98).

Files with missing lines Patch % Lines
...zation-invite-link/src/invite_link_admin_client.rs 71.53% 119 Missing ⚠️
...organization-invite-link/src/invite_link_client.rs 54.54% 25 Missing ⚠️
...ization-invite-link/src/invite_link_user_client.rs 99.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1461      +/-   ##
==========================================
- Coverage   86.16%   86.08%   -0.08%     
==========================================
  Files         533      535       +2     
  Lines       79699    80082     +383     
==========================================
+ Hits        68671    68938     +267     
- Misses      11028    11144     +116     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@quexten

quexten commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

All new functions, and new wrappers around existing functions (create and refresh), now return an OrganizationInviteLinkView. This handles the decryption of the invite secret and the construction of the URL all within the SDK. The client then just receives the constructed URL without having to worry about individual fields like Code or secret. @quexten one nice consequence of this is that the Invite struct won't need to pass the FFI boundary.

In my opinion, getting the web vault URL is a service that should be provided by the InternalClient

@quexten quexten left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The split to admin / user makes sense and looks pretty nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:tech-debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants