Skip to content

[PM-40531] feat: Lock the "Who can view" selection when enforced by policy - #7266

Open
andrebispo5 wants to merge 7 commits into
mainfrom
PM-40531-lock-who-can-view-when-enforced
Open

[PM-40531] feat: Lock the "Who can view" selection when enforced by policy#7266
andrebispo5 wants to merge 7 commits into
mainfrom
PM-40531-lock-who-can-view-when-enforced

Conversation

@andrebispo5

@andrebispo5 andrebispo5 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

When an org's SendControls policy sets whoCanAccess, the "Who can view" chooser on the Add Send
screen starts on the enforced option and can't be changed.

What changed

  • Two policy values restrict the chooser, and both map to an option that already exists:
    SPECIFIC_PEOPLE → Specific people, PASSWORD_PROTECTED → Anyone with a password set by you
  • ANY leaves every option available, so it is not a restriction. The chooser stays interactive,
    same as no policy at all
  • Lock only, no new options and no new strings
  • Email and password sub-fields stay editable while the chooser is locked

Why the ViewModel, not the chooser

  • The ticket's breakdown suggested forcing the value in AddEditSendAuthTypeChooser
  • Doing it there would let the chooser show one thing while state held another, and state is what
    gets saved
  • The chooser already reads its selection from state, so its side of this is a one-line isEnabled
    change

Out of scope

  • Existing Sends keep the access type they were created with. Non-compliant items get handled
    separately, when the edit option is disabled for them
  • Supporting text is unchanged, so no "enforced by your organization" message

Worth flagging

  • Dropping the enforcement mid-screen re-enables the picker and leaves the selection and anything
    typed alone. PM-40532 reset the deletion date to its default instead; resetting here would throw
    away entered emails or a password for no reason
  • Specific people is forced even without premium, since the policy outranks the premium gate that
    normally blocks picking it by hand

📸 Screenshots

Before After

@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development labels Aug 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.93%. Comparing base (9444b5f) to head (af5320d).

Files with missing lines Patch % Lines
...tools/feature/send/addedit/AddEditSendViewModel.kt 88.23% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7266      +/-   ##
==========================================
- Coverage   86.25%   85.93%   -0.33%     
==========================================
  Files         891      937      +46     
  Lines       65294    67225    +1931     
  Branches     9808     9861      +53     
==========================================
+ Hits        56320    57769    +1449     
- Misses       5472     5937     +465     
- Partials     3502     3519      +17     
Flag Coverage Δ
app-data 17.53% <0.00%> (-0.35%) ⬇️
app-ui-auth-tools 18.80% <90.00%> (+0.04%) ⬆️
app-ui-platform 16.89% <0.00%> (+0.47%) ⬆️
app-ui-vault 27.85% <0.00%> (+0.53%) ⬆️
authenticator 6.08% <0.00%> (-0.01%) ⬇️
lib-core-network-bridge 4.10% <0.00%> (-0.01%) ⬇️
lib-data-ui 1.20% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@andrebispo5
andrebispo5 marked this pull request as ready for review August 14, 2026 13:38
@andrebispo5
andrebispo5 requested review from a team and david-livefront as code owners August 14, 2026 13:38
Copilot AI lite review requested due to automatic review settings August 14, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Implements SendControls policy enforcement for the “Who can view” selection on the Add Send screen by forcing the initial auth/access type in the ViewModel and locking the chooser UI when an enforced policy applies.

Changes:

  • Enforce initial sendAuth based on EffectiveSendPolicy.whoCanAccess when SendControls is enabled (add mode only).
  • Lock the “Who can view” chooser when an enforced access type applies, while keeping email/password sub-fields editable.
  • Add/expand ViewModel and Compose UI tests covering enforced, ANY, and disabled/unset policy scenarios.

Reviewed changes

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

Show a summary per file
File Description
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt Enforces access type in add mode based on policy; exposes enforcedWhoCanAccess helper for UI locking.
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/components/AddEditSendAuthTypeChooser.kt Disables the chooser when enforcedWhoCanAccess is non-null.
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreen.kt Threads enforcedWhoCanAccess from state into content UI.
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt Accepts/passes enforcedWhoCanAccess down to the chooser.
app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModelTest.kt Adds unit tests for initial enforcement and policy-change behavior.
app/src/test/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendScreenTest.kt Adds Compose UI tests for locked/unlocked chooser behavior under policy.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1046 to +1047
val enforcedWhoCanAccess: SendAccessTypeJson?
get() = whoCanAccess?.takeIf { isSendControlsEnabled && it.restrictsAccess }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Intentional. AddEditSendCustomDateChooser (AddEditSendContent.kt:155) locks in edit mode the same way without an isAddMode gate, so this matches the existing pattern. Gating it would also let a compliant existing Send be switched to a type the policy forbids. Non-compliant existing Sends get edit disabled in a follow-up ticket.

Comment on lines +1720 to +1735
composeTestRule
.onNodeWithTag("SendAuthTypeChooser")
.performScrollTo()
.performClick()

// The enforced option is displayed, but the other options are never composed.
composeTestRule
.onNodeWithText("Anyone with the link")
.assertDoesNotExist()
composeTestRule
.onNodeWithText("Anyone with a password set by you")
.assertDoesNotExist()
composeTestRule
.onNodeWithText("Specific people")
.performScrollTo()
.assertIsDisplayed()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added assertIsNotEnabled() on the chooser, good call, the disabled state was not pinned before. Keeping the assertDoesNotExist() assertions too: "the dialog does not open" is the actual requirement, and removing the lock does make them fail, so they are not brittle.

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

Labels

app:password-manager Bitwarden Password Manager app context t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants