[AC-2274] Restrict Admin POST/PUT/DELETE Cipher Endpoints for V1 FC #3879

shane-melton · 2024-03-08T01:19:09Z

Type of change

- [ ] Bug fix
- [X] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

#3676 focused on adding new endpoints to restrict admins from reading all organization ciphers when the AllowAdminAccessToAllCollectionItems setting is disabled. This prevents admins from editing/deleting ciphers when that setting is disabled.

Code changes

`CanEditAnyCipherAsAdminAsync()` Helper

Creates a new helper to determine if the current user has admin capabilities based on the new logic defined in Flexible Collections V1. If Flexible Collections has not been enabled for the organization or the V1 flag is disabled fall back to current behavior (_currentContext.EditAnyCollections(orgId);).

All instances of _currentContext.EditAnyCollections(orgId) are then replaced with the new helper.

Before you submit

Please check for formatting errors (dotnet format --verify-no-changes) (required)
If making database changes - make sure you also update Entity Framework queries and/or migrations
Please add unit tests where it makes sense to do so (encouraged but not required)
If this change requires a documentation update - notify the documentation team
If this change has particular deployment requirements - notify the DevOps team

…ditAnyCollection usage

codecov · 2024-03-08T01:28:59Z

Codecov Report

Attention: Patch coverage is 26.19048% with 31 lines in your changes are missing coverage. Please review.

Project coverage is 38.02%. Comparing base (78b57ba) to head (582e657).

Files	Patch %	Lines
src/Api/Vault/Controllers/CiphersController.cs	26.19%	31 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3879      +/-   ##
==========================================
+ Coverage   37.83%   38.02%   +0.18%     
==========================================
  Files        1192     1192              
  Lines       57958    57981      +23     
  Branches     5549     5555       +6     
==========================================
+ Hits        21930    22045     +115     
+ Misses      34995    34896      -99     
- Partials     1033     1040       +7

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

gbubemismith

LGTM

shane-melton · 2024-03-12T23:00:03Z

@gbubemismith I had to undo the change to the Purge endpoints as Owners should still be able to purge an organization vault regardless of the collection management setting.

github-actions · 2024-03-28T23:51:57Z

Checkmarx One – Scan Summary & Details – 3d7b9a13-4ba6-4a8d-b157-bc77467df8d2

Fixed Issues

Severity	Issue	Source File / Package
	CSRF	/src/Api/AdminConsole/Controllers/ProvidersController.cs: 141
	CSRF	/src/Admin/AdminConsole/Controllers/ProvidersController.cs: 284
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 145
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 145
	CSRF	/src/Api/Billing/Controllers/ProviderClientsController.cs: 30
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 563
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 563
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 563
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 563
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 318
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 318
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 702
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 678
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 891
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 173
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 744
	CSRF	/src/Api/Vault/Controllers/FoldersController.cs: 45
	CSRF	/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 51
	CSRF	/src/Api/Controllers/UsersController.cs: 22
	CSRF	/src/Api/Controllers/DevicesController.cs: 70
	CSRF	/src/Api/Controllers/DevicesController.cs: 57
	CSRF	/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs: 69
	CSRF	/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs: 49
	CSRF	/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
	CSRF	/src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 92
	CSRF	/src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 49
	CSRF	/src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 142
	CSRF	/src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs: 52
	CSRF	/src/Api/AdminConsole/Controllers/PoliciesController.cs: 148
	CSRF	/src/Api/AdminConsole/Controllers/PoliciesController.cs: 78
	CSRF	/src/Api/AdminConsole/Controllers/PoliciesController.cs: 61
	CSRF	/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs: 50
	CSRF	/src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 161
	CSRF	/src/Api/Auth/Controllers/EmergencyAccessController.cs: 159
	CSRF	/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs: 98
	CSRF	/bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs: 88
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 231
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 271
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 144
	CSRF	/src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 188
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 744
	CSRF	/src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 175
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 613
	CSRF	/src/Api/Controllers/CollectionsController.cs: 303
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 669
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 222
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 858
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 807
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 288
	CSRF	/src/Api/Controllers/CollectionsController.cs: 410
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 191
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 187
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 526
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 362
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 174
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 128
	CSRF	/bitwarden_license/src/Sso/Controllers/AccountController.cs: 97
	CSRF	/bitwarden_license/src/Sso/Controllers/AccountController.cs: 164
	CSRF	/src/Admin/AdminConsole/Controllers/OrganizationsController.cs: 308
	CSRF	/src/Admin/AdminConsole/Controllers/ProvidersController.cs: 207
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 77
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 114
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 230
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 331
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 85
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 203
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 285
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 303
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 286
	CSRF	/src/Api/Controllers/CollectionsController.cs: 374
	CSRF	/src/Api/Auth/Controllers/TwoFactorController.cs: 403
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 231
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 220
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 408
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 900
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 571
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 271
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 970
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 970
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 726
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 825
	CSRF	/src/Api/Controllers/SettingsController.cs: 36
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 188
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 993
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 993
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 583
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 583
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 150
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 150
	CSRF	/src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 133
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 586
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 433
	CSRF	/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
	CSRF	/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
	CSRF	/src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 159
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1027
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 369
	CSRF	/src/Api/Controllers/CollectionsController.cs: 303
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 688
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 313
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 758
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 205
	CSRF	/src/Api/Controllers/CollectionsController.cs: 410
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 191
	CSRF	/src/Api/Controllers/CollectionsController.cs: 323
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 526
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 220
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1047
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 959
	CSRF	/src/Api/Controllers/CollectionsController.cs: 374
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 944
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 944
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 539
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 539
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 59
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 127
	CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 560
	CSRF	/src/Api/Controllers/CollectionsController.cs: 156
	CSRF	/src/Api/Controllers/CollectionsController.cs: 187
	CSRF	/src/Api/Controllers/CollectionsController.cs: 196
	CSRF	/src/Api/Public/Controllers/CollectionsController.cs: 64
	CSRF	/src/Api/Tools/Controllers/ImportCiphersController.cs: 50
	CSRF	/src/Api/Tools/Controllers/ImportCiphersController.cs: 66
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 111
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 125
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 889
	CSRF	/src/Identity/Controllers/AccountsController.cs: 50
	CSRF	/src/Identity/Controllers/AccountsController.cs: 72
	CSRF	/src/Admin/AdminConsole/Controllers/ProviderOrganizationsController.cs: 38
	CSRF	/src/Api/Auth/Controllers/WebAuthnController.cs: 130
	CSRF	/src/Api/Auth/Controllers/WebAuthnController.cs: 101
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 626
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1047
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 89
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 116
	CSRF	/src/Api/Controllers/CollectionsController.cs: 374
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationDomainController.cs: 75
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 318
	CSRF	/src/Api/AdminConsole/Controllers/GroupsController.cs: 145
	CSRF	/src/Api/Auth/Controllers/WebAuthnController.cs: 59
	CSRF	/src/Api/Auth/Controllers/WebAuthnController.cs: 153

More results are available on AST platform

shane-melton added 3 commits March 7, 2024 16:22

[AC-2274] Introduce CanEditAnyCiphersAsAdminAsync helper to replace E…

0eae406

…ditAnyCollection usage

[AC-2274] Add unit tests for CanEditAnyCiphersAsAdmin helper

f3bfa40

[AC-2274] Add Jira ticket

3a8d909

shane-melton requested a review from a team as a code owner March 8, 2024 01:19

shane-melton requested a review from gbubemismith March 8, 2024 01:19

shane-melton mentioned this pull request Mar 8, 2024

[AC-1707] Restrict provider access to items #3881

Merged

gbubemismith previously approved these changes Mar 11, 2024

View reviewed changes

[AC-2274] Undo change to purge endpoint

fb5aa65

shane-melton dismissed gbubemismith’s stale review via fb5aa65 March 12, 2024 22:57

shane-melton requested a review from gbubemismith March 12, 2024 23:00

gbubemismith previously approved these changes Mar 25, 2024

View reviewed changes

shane-melton added 2 commits March 26, 2024 11:16

Merge branch 'main' into vault/ac-2274/restrict-admin-cipher-endpoints

3ba268f

[AC-2274] Update admin checks to account for unassigned ciphers

0fab3f8

shane-melton dismissed gbubemismith’s stale review via 0fab3f8 March 28, 2024 23:47

shane-melton requested a review from gbubemismith April 8, 2024 20:34

shane-melton added 2 commits April 8, 2024 13:35

Merge branch 'main' into vault/ac-2274/restrict-admin-cipher-endpoints

31ee6ca

Merge branch 'main' into vault/ac-2274/restrict-admin-cipher-endpoints

2ad8cdf

gbubemismith approved these changes Apr 11, 2024

View reviewed changes

bitwarden-devops-bot temporarily deployed to EU-QA Cloud April 22, 2024 13:20 Inactive

bitwarden-devops-bot temporarily deployed to EU-QA Cloud April 22, 2024 18:04 Inactive

Merge branch 'main' into vault/ac-2274/restrict-admin-cipher-endpoints

582e657

bitwarden-devops-bot temporarily deployed to EU-QA Cloud April 26, 2024 02:51 Inactive

shane-melton merged commit 8e7bd79 into main Apr 30, 2024
49 checks passed

shane-melton deleted the vault/ac-2274/restrict-admin-cipher-endpoints branch April 30, 2024 17:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AC-2274] Restrict Admin POST/PUT/DELETE Cipher Endpoints for V1 FC #3879

[AC-2274] Restrict Admin POST/PUT/DELETE Cipher Endpoints for V1 FC #3879

shane-melton commented Mar 8, 2024 •

edited

codecov bot commented Mar 8, 2024 •

edited

gbubemismith left a comment

shane-melton commented Mar 12, 2024

github-actions bot commented Mar 28, 2024 •

edited

[AC-2274] Restrict Admin POST/PUT/DELETE Cipher Endpoints for V1 FC #3879

[AC-2274] Restrict Admin POST/PUT/DELETE Cipher Endpoints for V1 FC #3879

Conversation

shane-melton commented Mar 8, 2024 • edited

Type of change

Objective

Code changes

CanEditAnyCipherAsAdminAsync() Helper

Before you submit

codecov bot commented Mar 8, 2024 • edited

Codecov Report

gbubemismith left a comment

Choose a reason for hiding this comment

shane-melton commented Mar 12, 2024

github-actions bot commented Mar 28, 2024 • edited

Fixed Issues

shane-melton commented Mar 8, 2024 •

edited

`CanEditAnyCipherAsAdminAsync()` Helper

codecov bot commented Mar 8, 2024 •

edited

github-actions bot commented Mar 28, 2024 •

edited