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-394] Secrets Manager #2164

Merged
merged 41 commits into from
Jan 13, 2023
Merged

[SM-394] Secrets Manager #2164

merged 41 commits into from
Jan 13, 2023

Conversation

Hinton
Copy link
Member

@Hinton Hinton commented Aug 3, 2022

Objective

Secrets Manager

Type of change

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

Before you submit

- [ ] I have checked for formatting errors (`dotnet format --verify-no-changes`) (required)
- [ ] If making database changes - I have also updated Entity Framework queries and/or migrations
- [ ] I have added **unit tests** where it makes sense to do so (encouraged but not required)
- [ ] This change requires a **documentation update** (notify the documentation team)
- [ ] This change has particular **deployment requirements** (notify the DevOps team)

Objective
The purpose of this PR is to create a database table, entity, and repository for the new Secret database table.

The new Secret table will use entity framework for all database providers.
@Hinton Hinton added the ee label Aug 5, 2022
@mimartin12 mimartin12 added ee and removed ee labels Aug 5, 2022
Add a controller to fetch secrets associated with an organization ID.

To note, the [SecretsManager] attribute makes this controller only available for local development.
@mimartin12 mimartin12 added ee and removed ee labels Aug 17, 2022
…2201)

The purpose of this PR is to add API endpoints for getting, creating, and editing secrets for the Secrets Manager project.
@Hinton Hinton added ee and removed ee labels Aug 23, 2022
@mimartin12 mimartin12 added ee and removed ee labels Aug 24, 2022
Thomas-Avery and others added 4 commits August 29, 2022 08:59
…n date (#2206)

* Read UTC DateTimes from db and order by revision

* Move orderby to repo layer
…feature

# Conflicts:
#	bitwarden-server.sln
#	bitwarden_license/src/Scim/packages.lock.json
#	bitwarden_license/src/Sso/packages.lock.json
#	bitwarden_license/test/Commercial.Core.Test/packages.lock.json
#	src/Admin/packages.lock.json
#	src/Api/packages.lock.json
#	src/Billing/packages.lock.json
#	src/Events/packages.lock.json
#	src/EventsProcessor/packages.lock.json
#	src/Icons/packages.lock.json
#	src/Identity/packages.lock.json
#	src/Infrastructure.EntityFramework/packages.lock.json
#	src/Notifications/packages.lock.json
#	src/SharedWeb/packages.lock.json
#	test/Api.Test/packages.lock.json
#	test/Billing.Test/packages.lock.json
#	test/Common/packages.lock.json
#	test/Core.Test/packages.lock.json
#	test/Icons.Test/packages.lock.json
#	test/Identity.IntegrationTest/packages.lock.json
#	test/Identity.Test/packages.lock.json
#	test/IntegrationTestCommon/packages.lock.json
#	util/MySqlMigrations/packages.lock.json
#	util/PostgresMigrations/packages.lock.json
@mimartin12 mimartin12 added ee and removed ee labels Aug 31, 2022
cd-bitwarden and others added 12 commits September 5, 2022 11:18
* Project DB addition and sprocs

* Adding spaces to the end of each file, fixing minor issues

* removing useless comments

* Adding soft delete proc to migration

* Project EF Scaffold

* Additional changes to use EF instead of procedures

* Adding dependency injection

* Fixing lint errors

* Bug fixes

* Adding migration scripts, removing sproc files, and setting up Entity framework code

* Adding back accidentally deleted sproc

* Removing files that shouldn't have been created

* Lint

* Small changes based on Oscar's rec (#2215)

* Migrations for making CreateDate not null

* adding space to end of file

* Making Revision date not null

* dotnet format

* Adding nonclustered indexes to SQL

* SM-104: Update PR with changes Thomas proposed

Co-authored-by: CarleyDiaz-Bitwarden <[email protected]>
Co-authored-by: Thomas Avery <[email protected]>
Co-authored-by: Colton Hurst <[email protected]>
* SM-114: Initial commit with create project endpoint (for SM)

* SM-114: Add Update Project route (for SM)

* SM-114: Fix file encodings

* Fix DI issue for SM Project Create/Update commands

* Fix import ordering for linter

* SM-114: Remove unneeded lines setting DeletedDate, as it should already be null

* SM-114: Only have OrgId in route for CreateProject

* Remove unneeded using

* SM-114: Initial commit with create project endpoint (for SM)

* SM-114: Add Update Project route (for SM)

* SM-114: Fix file encodings

* Fix DI issue for SM Project Create/Update commands

* Fix import ordering for linter

* SM-114: Remove unneeded lines setting DeletedDate, as it should already be null

* SM-114: Only have OrgId in route for CreateProject

* Remove unneeded using

* Fully remove OrgId from ProjectCreateRequestModel
* Bulk delete secrets with command unit tests

* Controller unit tests

* Optimize conditionals

* SM-64 bulk delete integration test

* fix test

* SM-64 code review updated
* Adding integration tests for the SecretsController

Co-authored-by: Hinton <[email protected]>
* SM-114: Initial commit with create project endpoint (for SM)

* SM-114: Add Update Project route (for SM)

* SM-114: Fix file encodings

* Fix DI issue for SM Project Create/Update commands

* Adding GetProjectsByOrg

* fixing merge conflicts

* fix

* Updating to return empty list

* removing null check

Co-authored-by: Colton Hurst <[email protected]>
Co-authored-by: CarleyDiaz-Bitwarden <[email protected]>
* SM-191 Create ServiceAccount Table
…feature

# Conflicts:
#	bitwarden_license/src/Commercial.Core/Utilities/ServiceCollectionExtensions.cs
#	src/Api/Startup.cs
#	src/Api/Utilities/SecretsManagerAttribute.cs
#	src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs
#	src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs
#	src/SharedWeb/Utilities/ServiceCollectionExtensions.cs
#	test/Api.IntegrationTest/Factories/ApiApplicationFactory.cs
#	test/IntegrationTestCommon/Factories/WebApplicationFactoryBase.cs
#	util/PostgresMigrations/Factories.cs
* SM-96: Small change to allow getting project by id

* Fix whitespace issue

* Add first integration test and fix date bug

* Ensure tests are consistent

* Add more project controller integration tests

* Remove commented delete for now
Hinton and others added 19 commits October 13, 2022 18:01
* Beggining of changes for Project Secrets mapping

* Beggining of changes for project and secrets mapping

* Inital changes to add Mapping table for Project Secrets

* Resolve migration not working properly

* Indent sql

* Changes to try and return projects in the GetManyByOrganizaationIDAsync on SecretRepository.

* Changes made with Oscar

* Add reversemap

* running lint and removing comments

* Lint fixes

* fixing merge issues

* Trying to fix the DB issue

* DB fixes

* fixes

* removing unused space

* fixing lint issue

* final lint fix I hope

* removing manually added sql.sqlproj

* Lint changes and fixing the sql proj issues

* adding ServiceAccount to sql proj

* Removing ON DELETE CASCADE

* remove On delete cascade

* changes for deleting project and secret inside of the Organization_DeleteById procedure.

* changes for deleting project and secret inside of the Organization_DeleteById procedure.

* migration changes

* Updating constraints

* removing void

* remove spaces

* updating cipherRepo tests to be task instead of void

* fixing

* fixing

* test

* fix

* fix

* changes to remove circular dependency

* fixes

* sending guid and string name of the project over

* Update src/Sql/dbo/Tables/Secret.sql

Co-authored-by: Oscar Hinton <[email protected]>

* Update src/Sql/dbo/Tables/Project.sql

Co-authored-by: Oscar Hinton <[email protected]>

* removing unused code

* Potential refactor (#2340)

* migrations

* Postgres migraiton

* Update src/Api/SecretManagerFeatures/Models/Response/SecretResponseModel.cs

Co-authored-by: Oscar Hinton <[email protected]>

* rename file

* Update util/Migrator/DbScripts/2022-09-19_00_ProjectSecret.sql

Co-authored-by: Oscar Hinton <[email protected]>

* Lint fixes

* removing extra semi colon

* removing circular references with projects and secrets

* adding back projects

* Add ProjectFixture

* Update util/Migrator/DbScripts/2022-09-19_00_ProjectSecret.sql

Co-authored-by: Oscar Hinton <[email protected]>

* Update util/Migrator/DbScripts/2022-09-19_00_ProjectSecret.sql

Co-authored-by: Oscar Hinton <[email protected]>

Co-authored-by: CarleyDiaz-Bitwarden <[email protected]>
Co-authored-by: Hinton <[email protected]>
* Add create access token for service accounts
* Fix namespace for ServiceAccount command tests

* Remove "this" from SecretsManager requests

* Fix have scope be assigned a JSON list
* Update database to support never expiring ApiKey

* Update Api to support never expiring ApiKeys

* Fix unit test variable naming

* Remove required from model

* Fix spacing

* Add EF migrations

* Run dotnet format

* Update util/Migrator/DbScripts/2022-11-29_00_ApiKey_Never_Expire.sql

Co-authored-by: Oscar Hinton <[email protected]>

Co-authored-by: Oscar Hinton <[email protected]>
* Fix and re-enable parallel integration tests

* Fix package lock files

* Move fix to ApiApplicationFactory

* Run dotnet restore --force

* Run dotnet format

* Reset packages.lock.json files
* Add GetSecretsByProjectAsync endpoint

* Add GetManyByProjectIdAsync endpoint

* Update response model for GetSecretsByProjectAsync

* Include projects when returning secrets by project id

* SM-99: Add ability to specify projectId when creating a secret

* SM-99: Update tests to accomodate for new create secret parameter

* Fix failing test

* SM-99: Handle optional projectId for new secret in ToSecret()

* SM-99: Filter out deleted secrets on GetManyByProjectIdAsync() and small refactorings

* SM-99: make CreateAsync for secret more clear

* Add CreateSecretWithProject integration test

* Fix CreateSecretWithProject integration test for SM-99

* Run dotnet format

* Undo added space

* Refactor test

* Refactor CreateSecretWithProject API Integration test again
* testing

* test2

* testing

* trying to save the projects associated with the secret

* changes

* more changes

* Fix  EF error

* Second attempt

* Replace AddIfNotExists with Add.

* changes

* fixing await issue

* lint

* lint fixes

* suggested changes

* suggested changes

* updating tests

* fixing tests 2

* fixing tests

* fixing test

* fixing test

* fixing tests

* test

* testing

* fixing tests for the millionth time

* fixing tests

* allowing nulls for projectIds, fixing lint

* fixing tests

Co-authored-by: Hinton <[email protected]>
…feature

# Conflicts:
#	bitwarden_license/src/Scim/packages.lock.json
#	bitwarden_license/src/Sso/packages.lock.json
#	bitwarden_license/test/Scim.IntegrationTest/packages.lock.json
#	src/Admin/Views/Organizations/Edit.cshtml
#	src/Admin/packages.lock.json
#	src/Api/packages.lock.json
#	src/Billing/packages.lock.json
#	src/Events/packages.lock.json
#	src/EventsProcessor/packages.lock.json
#	src/Icons/packages.lock.json
#	src/Identity/IdentityServer/CustomTokenRequestValidator.cs
#	src/Identity/packages.lock.json
#	src/Infrastructure.EntityFramework/Infrastructure.EntityFramework.csproj
#	src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs
#	src/Infrastructure.EntityFramework/packages.lock.json
#	src/Notifications/packages.lock.json
#	src/SharedWeb/Utilities/ServiceCollectionExtensions.cs
#	src/SharedWeb/packages.lock.json
#	src/Sql/dbo/Stored Procedures/Organization_Create.sql
#	src/Sql/dbo/Stored Procedures/Organization_Update.sql
#	src/Sql/dbo/Tables/Organization.sql
#	test/Api.IntegrationTest/packages.lock.json
#	test/Api.Test/packages.lock.json
#	test/Billing.Test/packages.lock.json
#	test/Icons.Test/packages.lock.json
#	test/Identity.IntegrationTest/packages.lock.json
#	test/Identity.Test/packages.lock.json
#	test/Infrastructure.EFIntegration.Test/Repositories/CipherRepositoryTests.cs
#	test/Infrastructure.EFIntegration.Test/packages.lock.json
#	test/IntegrationTestCommon/packages.lock.json
#	util/MySqlMigrations/packages.lock.json
#	util/PostgresMigrations/Migrations/DatabaseContextModelSnapshot.cs
#	util/PostgresMigrations/packages.lock.json
* Fix tables not being cleaned up

* Fix migration

* Squash secrets manager migrations

* Reset EF to pre SM state

* Add EF migrations

* Fix unified docker

* Add missed copy
…feature

# Conflicts:
#	util/MySqlMigrations/HelperScripts/2023-01-06_00_SecretsManager.sql
#	util/PostgresMigrations/HelperScripts/2023-01-06_00_SecretsManager.psql
#	util/SqliteMigrations/Migrations/DatabaseContextModelSnapshot.cs
* Bump EF.SqlServer version to match EF

* Rename AddCommercialSecretsServices to AddCommercialSecretsManagerServices

* Fix format of ServiceAccountRepository

* Delete sql migration files

* Fix build warnings

* Remove duplicate LinqToDBForEfTools.Initialize
@Hinton Hinton marked this pull request as ready for review January 11, 2023 16:21
@Hinton Hinton requested a review from coltonhurst January 11, 2023 16:21
Copy link
Member

@coltonhurst coltonhurst left a comment

Choose a reason for hiding this comment

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

@Hinton thanks for putting this together. I've gone through each commit & followed the review guidelines based on our long-lived feature branch documentation. Everything looks good, I just want to verify if the following have been tested or if we need to wait for testing to complete:

  • SM-185
  • SM-246
  • SM-218
  • SM-282
  • SM-259
  • SM-361
  • SM-394
  • SM-300
  • SM-73

You can find links to the commit, PR, and ticket in the internal excel doc I put together. Please let me know what we might need to wait for on the above, and if we are good if some of these haven't had testing, etc.

Finally, how can we confirm & ensure everything is behind the proper feature flag? I believe everything should be, but we should double check.

@Hinton Hinton changed the title [SM-60] Secrets [SM-394] Secrets Manager Jan 13, 2023
@Hinton
Copy link
Member Author

Hinton commented Jan 13, 2023

All the endpoints should be behind the [SecretsManager] attribute which for this use case is our feature flag. We'll do the final testing once it's merged to master.

Copy link
Member

@coltonhurst coltonhurst left a comment

Choose a reason for hiding this comment

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

Everything looks good, thank you team for the hard work on this!

@Hinton Hinton merged commit 1f0fc43 into master Jan 13, 2023
@Hinton Hinton deleted the sm/secrets-feature branch January 13, 2023 14:02
@bw-ephemeral-env-bot bw-ephemeral-env-bot bot removed the ee label Jan 13, 2023
@Hinton Hinton restored the sm/secrets-feature branch January 16, 2023 14:34
@Hinton Hinton deleted the sm/secrets-feature branch July 11, 2023 14:33
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.

5 participants