[PM-11343] Browser Refresh - View dialog permissions in AC #11092

shane-melton · 2024-09-16T23:39:19Z

🎟️ Tracking

PM-11343 - Edit button never available in Individual Vault
PM-11346 - Edit button not available to admins with permissions to edit all items
PM-11337 - Unable to open view dialog for items belonging to Unassigned or collections not assigned to

📔 Objective

Update the View dialog to support the unique behavior of the AC which has different data loading strategies for items/collections that are only applicable to admins.

The "Edit" button should always be enabled when viewing a cipher in the Individual Vault as that is the only way to Favorite an item or move it to a new folder. When in the AC, the "Edit" button can be disabled if the user does not have edit permissions as users are not expected to interact with favorites/folders in the AC.

The AC must explicitly override the collections for items they do not normally have access to so that they can be rendered in the Cipher View component.

📸 Screenshots

Edit buttons in Individual Vault and AC

Screen.Recording.2024-09-16.at.4.37.51.PM.mov

Accessing Unassigned items

Screen.Recording.2024-09-18.at.12.19.13.PM.mov

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

… callers of the View dialog

shane-melton · 2024-09-16T23:39:47Z

apps/web/src/app/vault/individual-vault/vault.component.ts

@@ -622,10 +622,6 @@ export class VaultComponent implements OnInit, OnDestroy {
    component.folderId = this.activeFilter.folderId;
  }

-  async navigateToCipher(cipher: CipherView) {


ℹ️ This was not used anywhere.

shane-melton · 2024-09-16T23:41:07Z

apps/web/src/app/vault/individual-vault/view.component.ts

@@ -137,13 +139,6 @@ export class ViewComponent implements OnInit, OnDestroy {
   */
  async edit(): Promise<void> {
    this.dialogRef.close({ action: ViewCipherDialogResult.edited });
-    await this.router.navigate([], {


ℹ️ Letting the calling component handle the router makes it easier to share the component between the AC and Individual Vault. (e.g. the AC does not need to update the organizationId parameter). Also, should we ever move away from requiring navigation to open the dialogs, this is one less place to track.

github-actions · 2024-09-16T23:49:59Z

Checkmarx One – Scan Summary & Details – 3562b331-e3a8-4c56-81b1-57843cac53e4

No New Or Fixed Issues Found

…43/view-dialog-edit-button-permissions

codecov · 2024-09-17T18:19:23Z

Codecov Report

Attention: Patch coverage is 17.24138% with 24 lines in your changes missing coverage. Please review.

Project coverage is 35.07%. Comparing base (afff91e) to head (918cfa5).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...pps/web/src/app/vault/org-vault/vault.component.ts	0.00%	17 Missing ⚠️
.../src/app/vault/individual-vault/vault.component.ts	0.00%	4 Missing ⚠️
...ibs/vault/src/cipher-view/cipher-view.component.ts	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11092      +/-   ##
==========================================
- Coverage   35.08%   35.07%   -0.01%     
==========================================
  Files        2709     2710       +1     
  Lines       84531    84545      +14     
  Branches    16068    16068              
==========================================
  Hits        29655    29655              
- Misses      53905    53919      +14     
  Partials      971      971

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

…43/view-dialog-edit-button-permissions

…e to closing via the ESC key)

… view component

…he View cipher dialog

…back button

shane-melton · 2024-09-18T19:05:11Z

apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html

@@ -21,6 +21,7 @@
      [routerLink]="[]"
      [queryParams]="{ itemId: cipher.id, action: extensionRefreshEnabled ? 'view' : null }"
      queryParamsHandling="merge"
+      [replaceUrl]="extensionRefreshEnabled"


ℹ️ This fixes a problem where after closing a dialog, you'd have to hit the browser's back button twice to actually navigate back. The same problem exists in production currently, but its behind the flag in case it introduces unexpected behavior in the old modals.

shane-melton · 2024-09-18T19:07:50Z

libs/vault/src/cipher-view/cipher-view.component.ts

+   * Optional list of collections the cipher is assigned to. If none are provided, they will be fetched using the
+   * `CipherService` and the `collectionIds` property of the cipher.
+   */
+  @Input() collections: CollectionView[];


ℹ️ This override is necessary because in the Admin console, an admin might not have "normal" access to a cipher and its collections. When this happens the collectionService.decryptedCollectionViews$ will return empty so the Admin Console needs a means to provide collections it has retrieved via the Admin endpoints.

shane-melton · 2024-09-18T19:10:29Z

apps/web/src/app/vault/org-vault/vault.component.ts

-    ) {
-      // didn't pass password prompt, so don't open add / edit modal.
+  async viewCipher(cipher: CipherView, collections: CollectionView[] = []) {
+    if (!cipher) {


ℹ️ Updated to use view objects instead of Ids to avoid needing to refetch the cipher and collection via admin endpoints. The component already handles determining which ciphers/collections a user has access to in the observable setup above, so instead of re-implementing that here and making more API requests, we just accept the view objects themselves.

…43/view-dialog-edit-button-permissions

shane-melton added 2 commits September 16, 2024 16:24

[PM-11343] Add param to conditionally disable the edit button

1f9d611

[PM-11343] Cleanup router navigation and move query param handling to…

e5fc71a

… callers of the View dialog

shane-melton requested a review from a team as a code owner September 16, 2024 23:39

shane-melton requested a review from cd-bitwarden September 16, 2024 23:39

shane-melton commented Sep 16, 2024

View reviewed changes

shane-melton requested a review from alec-livefront September 17, 2024 00:07

shane-melton added 2 commits September 17, 2024 11:10

[PM-11343] Fix failing test

0c40d9b

Merge branch 'main' of github.com:bitwarden/clients into vault/pm-113…

6fb925f

…43/view-dialog-edit-button-permissions

shane-melton requested a review from Jingo88 September 17, 2024 19:48

shane-melton added 2 commits September 17, 2024 14:57

Merge branch 'main' of github.com:bitwarden/clients into vault/pm-113…

637d53c

…43/view-dialog-edit-button-permissions

[PM-11343] Fix missing router after merge

c78f44e

shane-melton marked this pull request as draft September 18, 2024 17:56

Merge branch 'main' of github.com:bitwarden/clients into vault/pm-113…

2bdd14a

…43/view-dialog-edit-button-permissions

shane-melton force-pushed the vault/pm-11343/view-dialog-edit-button-permissions branch from fe5d3c0 to 2bdd14a Compare September 18, 2024 18:03

shane-melton added 6 commits September 18, 2024 11:30

[PM-11343] Add null checks in case the dialog result is undefined (du…

8c0e3f1

…e to closing via the ESC key)

[PM-11343] Add support to provide a list of collections to the cipher…

4e33f18

… view component

[PM-11343] Add collections as an optional view cipher dialog parameter

8061beb

[PM-11343] Update the org vault to provide collections when opening t…

7c96500

…he View cipher dialog

[PM-11343] Fix import

ff6f0dc

[PM-11343] Use [replaceUrl] for cipher items to avoid needing double …

2808001

…back button

shane-melton commented Sep 18, 2024

View reviewed changes

shane-melton changed the title ~~[PM-11343] Browser Refresh - View dialog edit button permissions~~ [PM-11343] Browser Refresh - View dialog permissions in AC Sep 18, 2024

Merge branch 'main' of github.com:bitwarden/clients into vault/pm-113…

7fe925f

…43/view-dialog-edit-button-permissions

shane-melton marked this pull request as ready for review September 18, 2024 19:22

Merge branch 'main' of github.com:bitwarden/clients into vault/pm-113…

918cfa5

…43/view-dialog-edit-button-permissions

Jingo88 previously approved these changes Sep 19, 2024

View reviewed changes

shane-melton dismissed Jingo88’s stale review via 918cfa5 September 19, 2024 16:44

shane-melton requested a review from Jingo88 September 19, 2024 17:17

shane-melton mentioned this pull request Sep 19, 2024

[PM-5450] Admin Console event collection #10678

Merged

Jingo88 approved these changes Sep 19, 2024

View reviewed changes

shane-melton merged commit 4327fa2 into main Sep 19, 2024
66 checks passed

shane-melton deleted the vault/pm-11343/view-dialog-edit-button-permissions branch September 19, 2024 17:43

shane-melton mentioned this pull request Sep 23, 2024

[PM-12528] AC Fix Collection Refresh #11207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-11343] Browser Refresh - View dialog permissions in AC #11092

[PM-11343] Browser Refresh - View dialog permissions in AC #11092

shane-melton commented Sep 16, 2024 •

edited

Loading

shane-melton Sep 16, 2024

shane-melton Sep 16, 2024

github-actions bot commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

shane-melton Sep 18, 2024 •

edited

Loading

shane-melton Sep 18, 2024

shane-melton Sep 18, 2024 •

edited

Loading

[PM-11343] Browser Refresh - View dialog permissions in AC #11092

[PM-11343] Browser Refresh - View dialog permissions in AC #11092

Conversation

shane-melton commented Sep 16, 2024 • edited Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

Edit buttons in Individual Vault and AC

Accessing Unassigned items

⏰ Reminders before review

🦮 Reviewer guidelines

shane-melton Sep 16, 2024

Choose a reason for hiding this comment

shane-melton Sep 16, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 16, 2024 • edited Loading

No New Or Fixed Issues Found

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

shane-melton Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

shane-melton Sep 18, 2024

Choose a reason for hiding this comment

shane-melton Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

shane-melton commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

shane-melton Sep 18, 2024 •

edited

Loading

shane-melton Sep 18, 2024 •

edited

Loading