Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from May 7, 2024

Conversation

shane-melton
Copy link
Member

@shane-melton shane-melton commented Mar 8, 2024

Type of change

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

Objective

Restrict providers from modifying/deleting ciphers within their client organizations when the restrict-provider-access feature flag is enabled.

Depends on changes in #3879 being merged first.

Also depends on Client PR: bitwarden/clients#8265

Code changes

CiphersController.cs

Update CanEditAnyCipherAsAdminAsync() helper to check for restrict-provider-access feature flag when a provider user attempts to edit a cipher via the /admin cipher endpoints.

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

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

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

Project coverage is 38.33%. Comparing base (928f94d) to head (ee76e76).

Files Patch % Lines
src/Api/Vault/Controllers/CiphersController.cs 80.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3881      +/-   ##
==========================================
- Coverage   38.39%   38.33%   -0.07%     
==========================================
  Files        1209     1209              
  Lines       58677    58686       +9     
  Branches     5587     5589       +2     
==========================================
- Hits        22531    22495      -36     
- Misses      35103    35146      +43     
- Partials     1043     1045       +2     

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

LRNcardozoWDF
LRNcardozoWDF previously approved these changes Mar 12, 2024
Copy link
Member

@LRNcardozoWDF LRNcardozoWDF left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

github-actions bot commented Apr 10, 2024

Logo
Checkmarx One – Scan Summary & Detailsea57c42e-b77b-472c-8358-e56904491416

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 87 Attack Vector
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 270 Attack Vector
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 212 Attack Vector
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 270 Attack Vector
MEDIUM Privacy_Violation /src/Api/Auth/Controllers/AccountsController.cs: 955 Attack Vector
MEDIUM Privacy_Violation /src/Api/Auth/Controllers/AccountsController.cs: 655 Attack Vector
MEDIUM Privacy_Violation /src/Api/Vault/Controllers/CiphersController.cs: 939 Attack Vector
MEDIUM Privacy_Violation /src/Api/Auth/Controllers/AccountsController.cs: 518 Attack Vector
MEDIUM Privacy_Violation /src/Api/Auth/Controllers/AccountsController.cs: 937 Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 929 Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 647 Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 947 Attack Vector
LOW Log_Forging /src/Api/Auth/Controllers/AccountsController.cs: 510 Attack Vector
LOW Log_Forging /src/Api/Vault/Controllers/CiphersController.cs: 931 Attack Vector

Fixed Issues

Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 587
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 587
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 587
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 587
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 132
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProvidersController.cs: 141
MEDIUM CSRF /src/Api/SecretsManager/Controllers/AccessPoliciesController.cs: 229
MEDIUM CSRF /src/Admin/AdminConsole/Controllers/ProvidersController.cs: 309
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 161
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 161
MEDIUM CSRF /src/Api/Billing/Controllers/ProviderClientsController.cs: 30
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 190
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 331
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 331
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 710
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 686
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 891
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 173
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 752
MEDIUM CSRF /src/Api/Vault/Controllers/FoldersController.cs: 45
MEDIUM CSRF /src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 51
MEDIUM CSRF /src/Api/Controllers/UsersController.cs: 22
MEDIUM CSRF /src/Api/Controllers/DevicesController.cs: 70
MEDIUM CSRF /src/Api/Controllers/DevicesController.cs: 57
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/PoliciesController.cs: 69
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/PoliciesController.cs: 49
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 92
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 49
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 142
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs: 52
MEDIUM CSRF /src/Api/AdminConsole/Controllers/PoliciesController.cs: 148
MEDIUM CSRF /src/Api/AdminConsole/Controllers/PoliciesController.cs: 78
MEDIUM CSRF /src/Api/AdminConsole/Controllers/PoliciesController.cs: 61
MEDIUM CSRF /bitwarden_license/src/Sso/Controllers/AccountController.cs: 163
MEDIUM CSRF /bitwarden_license/src/Sso/Controllers/AccountController.cs: 96
MEDIUM CSRF /bitwarden_license/src/Scim/Controllers/v2/UsersController.cs: 50
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 161
MEDIUM CSRF /src/Api/Auth/Controllers/EmergencyAccessController.cs: 159
MEDIUM CSRF /bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs: 98
MEDIUM CSRF /bitwarden_license/src/Scim/Controllers/v2/GroupsController.cs: 88
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 222
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 222
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 288
MEDIUM CSRF /src/Api/Controllers/SettingsController.cs: 36
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 411
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 193
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 362
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 766
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 375
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1100
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 244
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 571
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 284
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 303
MEDIUM CSRF /src/Admin/AdminConsole/Controllers/OrganizationsController.cs: 308
MEDIUM CSRF /src/Admin/AdminConsole/Controllers/ProvidersController.cs: 232
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 81
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 118
MEDIUM CSRF /src/Identity/Controllers/AccountsController.cs: 72
MEDIUM CSRF /src/Identity/Controllers/AccountsController.cs: 50
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 230
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 331
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 942
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 125
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 86
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 216
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 111
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 298
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 316
MEDIUM CSRF /src/Api/Tools/Controllers/ImportCiphersController.cs: 66
MEDIUM CSRF /src/Api/Tools/Controllers/ImportCiphersController.cs: 50
MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 64
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 408
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 550
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 607
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 607
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 159
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 299
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 586
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 433
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 222
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 702
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 967
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1023
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1023
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 563
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 563
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 590
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 908
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 193
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 313
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 244
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 174
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 187
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 257
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 284
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 447
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 613
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 303
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 411
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 791
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 323
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 144
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 878
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 805
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1046
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1046
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 815
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 375
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 550
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 274
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 150
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 150
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 133
MEDIUM CSRF /src/Api/Auth/Controllers/TwoFactorController.cs: 403
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 175
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 911
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 728
MEDIUM CSRF

More results are available on AST platform

Base automatically changed from vault/ac-2274/restrict-admin-cipher-endpoints to main April 30, 2024 17:28
@shane-melton shane-melton dismissed LRNcardozoWDF’s stale review April 30, 2024 17:28

The base branch was changed.

Copy link
Contributor

github-actions bot commented May 2, 2024

LaunchDarkly flag references

🔍 1 flag added or modified

Name Key Aliases found Info
restrict-provider-access restrict-provider-access

@shane-melton shane-melton merged commit 45be4d5 into main May 7, 2024
49 checks passed
@shane-melton shane-melton deleted the vault/ac-1707/restrict-provider-access-to-items branch May 7, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants