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

Allow for bulk updating AuthRequest database objects #4053

Merged

Conversation

addisonbeck
Copy link
Contributor

@addisonbeck addisonbeck commented May 3, 2024

Type of change

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

🧪 NOTE
Despite what the bots want you to think: I did write an integration test for this.

Objective

To facilitate a new bulk device login request approval workflow in the admin
console we need to update IAuthRequestRepisitory to include an
UpdateManyAsync() method. It should accept a list of AuthRequest table
objects, and implementations will do a very simple 1:1 update of the passed
in data.

This pull request implements this behavior in the repository and database
layers of the bitwarden server.

Code changes

See the commits tab

References

  • AC-2301 is the Jira ticket for this work.
  • This PR is extended by #4064, which adds a command layer for applying business logic
    to the more general and high level functions written in this PR for the repository layer.
  • There is also #4077 which exposes an API endpoint that calls through to the methods implanted here.

@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch from bb61771 to c6974aa Compare May 3, 2024 22:08
@bitwarden bitwarden deleted a comment from codecov bot May 3, 2024
@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch 15 times, most recently from afe11b7 to d16f587 Compare May 6, 2024 22:33
To facilitate a new bulk device login request approval workflow in the
admin console we need to update `IAuthRequestRepisitory` (owned by Auth
team) to include an`UpdateManyAsync()` method. It should accept a list
of `AuthRequest` table objects, and implementations will do a very
simple 1:1 update of the passed in data.

This commit adds an `UpdateManyAsync()` method to the
`AuthRequestRepository` interface.
This commit stubs out implementations of
`IAuthRequestRepository.UpdateManyAsync()` so the method signature can
be called in unit tests. At this stage the methods are not implemented.
To facilitate a bulk update operation for auth requests a new user
defined type will need to be written that can be used as a table input
to the stored procedure. This will follow a similar pattern to how the
`OragnizationSponsorshipType` works and is used by the stored procedure
`OrganizationSponsorship_UpdateMany`.
To facilitate the bulk updating of auth request table objects this
commit adds a new stored procedure to  update a collection of entities
on `AuthRequest` table by their primary key. It updates all properties,
for convention, but the endpoint created later will only change the
`Approved`, `ResponseDate`, `Key`, `MasterPasswordHash`, and
`AuthenticationDate` properties.
This commit simply applies a migration script containing the new user
defined type and stored procedure comitted previously.
The current pattern in place for bulk update stored procedures is to
pass a `DataTable` through Dapper as an input for the update stored
procedure being run. In order to facilitate the new bulk update
procedure for the`AuthRequest` type we need a function added that can
convert an `IEnumerable<AuthRequest>` to a `DataTable`. This is commit
follows the convention of having a static class with a conversion method
in a `Helpers` folder: `AuthRequestHelpers.ToDataTable()`.
This commit implements `AuthRequestRepository.UpdateMany()` for the
Dapper implementation of `AuthRequestRepository`. It connects the stored
procedure, `DataTable` converter, and Dapper-focused unit test commits
written previously into one exposed method that can be referenced by
service callers.
This commit implements the new
`IAuthRequestRepository.UpdateManyAsync()`method in the Entity Framework
skew of the repository layer. It checks to make sure the passed in list
has auth requests, converts them all to an Entity Framework entity, and
then uses `UpdateRange` to apply the whole thing over in the database
context.
@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch from d16f587 to 9d5b27e Compare May 6, 2024 22:37
@bitwarden bitwarden deleted a comment from codecov bot May 6, 2024
@bitwarden bitwarden deleted a comment from github-actions bot May 6, 2024
Copy link
Contributor

github-actions bot commented May 6, 2024

Logo
Checkmarx One – Scan Summary & Details37c3868e-a42f-41b1-8b2d-5fd43ac2fd2f

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Privacy_Violation /src/Api/Controllers/DevicesController.cs: 155 Attack Vector
MEDIUM Privacy_Violation /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 343 Attack Vector
MEDIUM Privacy_Violation /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 396 Attack Vector
LOW Log_Forging /src/Api/Vault/Controllers/CiphersController.cs: 222 Attack Vector
LOW Log_Forging /src/Api/Controllers/DevicesController.cs: 146 Attack Vector
LOW Log_Forging /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 371 Attack Vector
LOW Log_Forging /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 308 Attack Vector

Fixed Issues

Severity Issue Source File / Package
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: 607
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 607
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: 320
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 163
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 163
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: 333
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 333
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 645
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 669
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 535
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 711
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/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: 287
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 570
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 407
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 432
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 144
MEDIUM CSRF /src/Admin/AdminConsole/Controllers/OrganizationsController.cs: 308
MEDIUM CSRF /src/Admin/AdminConsole/Controllers/ProvidersController.cs: 243
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 81
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 118
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 187
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/OrganizationController.cs: 42
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 323
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 722
MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 64
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 187
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 159
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 867
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 193
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 175
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 277
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 361
MEDIUM CSRF /src/Api/Tools/Controllers/ImportCiphersController.cs: 50
MEDIUM CSRF /src/Api/Tools/Controllers/ImportCiphersController.cs: 66
MEDIUM CSRF /src/Identity/Controllers/AccountsController.cs: 72
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 111
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 125
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 786
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 246
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 411
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 962
MEDIUM CSRF /src/Identity/Controllers/AccountsController.cs: 50
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 375
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1100
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1066
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1066
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 303
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 811
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProviderUsersController.cs: 188
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 627
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 627
MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 260
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 174
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 572
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 193
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1043
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1043
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 774
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 246
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 411
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 375
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1017
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1017
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 222
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 286
MEDIUM CSRF /src/Api/Controllers/SettingsController.cs: 36
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 898
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 825
MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 303
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 283
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 222
MEDIUM CSRF /src/Api/Auth/Controllers/TwoFactorController.cs: 403
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 926
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 117
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 283
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 86
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 218
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 583
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 300
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 318
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 583
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/GroupsController.cs: 133
MEDIUM CSRF /src/Api/AdminConsole/Controllers/ProviderOrganizationsController.cs: 53
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 530
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 449
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 545
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 283
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 283
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationsController.cs: 283
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 570
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1120
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 150
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 150
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 748
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 931
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 312
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 59
MEDIUM CSRF /src/Api/AdminConsole/Public/Controllers/MembersController.cs: 127
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 519
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 301
MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 286
MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 221
MEDIUM CSRF

More results are available on AST platform

@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch 5 times, most recently from 5d2c926 to fab5fde Compare May 13, 2024 19:20
@addisonbeck addisonbeck requested a review from a team as a code owner May 13, 2024 19:20
@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch from fab5fde to cea0589 Compare May 13, 2024 19:22
@addisonbeck addisonbeck removed the request for review from a team May 13, 2024 19:22
@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch 2 times, most recently from f68432e to f4d6438 Compare May 20, 2024 20:59
@addisonbeck addisonbeck force-pushed the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch from f4d6438 to 632e3fa Compare May 20, 2024 21:05
Copy link
Contributor

@rkac-bw rkac-bw left a comment

Choose a reason for hiding this comment

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

LGTM

@addisonbeck addisonbeck merged commit 56c523f into main May 22, 2024
53 checks passed
@addisonbeck addisonbeck deleted the ac/addison/ac-2301/service-bulk-device-approval-endpoint-api branch May 22, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants