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

[SM-949] Add endpoint to fetch events by service account #3336

Merged
merged 14 commits into from
Oct 19, 2023
Merged

Conversation

Thomas-Avery
Copy link
Contributor

@Thomas-Avery Thomas-Avery commented Oct 9, 2023

Type of change

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

Objective

The purpose of this PR is to add a Secrets Manager endpoint for fetching events by service account.

Code changes

  • bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandler.cs:
    bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs:
    src/Core/SecretsManager/AuthorizationRequirements/ServiceAccountOperationRequirement.cs:
    Add operation for the authz handler for reading service account events.

  • src/Api/Controllers/EventsController.cs:
    src/Api/Utilities/ApiHelpers.cs:
    Extract the private method GetDateRange into the ApiHelpers utility so it can be used in this controller and the SM controller.

  • src/Api/SecretsManager/Controllers/SecretsManagerEventsController.cs:
    Add a new controller for fetching Secrets Manager events.

  • src/Core/Repositories/IEventRepository.cs:
    Add a new method to fetch events by organization and service account.

  • src/Core/Repositories/TableStorage/EventRepository.cs:
    Add a new method to fetch events by organization and service account.

  • src/Infrastructure.Dapper/Repositories/EventRepository.cs:
    Add a new method to fetch events by organization and service account.
    Fix missing Secrets Manager columns in the create many events flow.

  • src/Infrastructure.EntityFramework/Repositories/EventRepository.cs:
    src/Infrastructure.EntityFramework/Repositories/Queries/EventReadPageByOrganizationIdServiceAccountIdQuery.cs:
    Add a new method and EF query to fetch events by organization and service account.

  • src/Sql/SecretsManager/dbo/Stored Procedures/Event/Event_ReadPageByOrganizationIdServiceAccountId.sql:
    Add stored procedure for reading service account events.

  • test/Api.IntegrationTest/SecretsManager/Controllers/SecretsManagerEventsControllerTests.cs:
    Add integration tests for new controller.

  • test/Api.Test/SecretsManager/Controllers/SecretsManagerEventsControllerTests.cs:
    Add unit tests for new controller.

  • util/Migrator/DbScripts/2023-10-09_00_Event_ReadPageByOrganizationIdServiceAccountId.sql:
    Migration to add the new stored procedure.

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

@Thomas-Avery Thomas-Avery self-assigned this Oct 9, 2023
@bitwarden-bot
Copy link

bitwarden-bot commented Oct 9, 2023

Logo
Checkmarx One – Scan Summary & Details78303a9b-4129-4d09-928b-28e9f821d7d0

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM CSRF /src/Api/SecretsManager/Controllers/SecretsManagerEventsController.cs: 31 Attack Vector

@Thomas-Avery Thomas-Avery marked this pull request as ready for review October 10, 2023 17:25
@Thomas-Avery Thomas-Avery requested review from a team as code owners October 10, 2023 17:25
cd-bitwarden
cd-bitwarden previously approved these changes Oct 10, 2023
Copy link
Contributor

@cd-bitwarden cd-bitwarden left a comment

Choose a reason for hiding this comment

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

LGTM 😄

vincentsalucci
vincentsalucci previously approved these changes Oct 13, 2023
Copy link
Member

@vincentsalucci vincentsalucci left a comment

Choose a reason for hiding this comment

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

Just a couple of small side conversations that should be non-blocking. 👍

vincentsalucci
vincentsalucci previously approved these changes Oct 16, 2023
cd-bitwarden
cd-bitwarden previously approved these changes Oct 16, 2023
Copy link
Contributor

@cd-bitwarden cd-bitwarden 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

@cd-bitwarden cd-bitwarden left a comment

Choose a reason for hiding this comment

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

changes look good 👍 😄

@Thomas-Avery Thomas-Avery merged commit 728cd1c into master Oct 19, 2023
43 checks passed
@Thomas-Avery Thomas-Avery deleted the sm/sm-949 branch October 19, 2023 21:57
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.

4 participants