Merge branch 'main' into ios/PM-23500/sourcery-usage

Author: fedemkr

.github/workflows/scan.yml

@@ -8,7 +8,7 @@ on:
   pull_request:
     types: [opened, synchronize, reopened]
     branches-ignore:
-      - main
+      - "main"
   pull_request_target:
     types: [opened, synchronize, reopened]
     branches:
@@ -24,68 +24,28 @@ jobs:
       contents: read
 
   sast:
-    name: SAST scan
-    runs-on: ubuntu-22.04
+    name: Checkmarx
+    uses: bitwarden/gh-actions/.github/workflows/_checkmarx.yml@main
     needs: check-run
+    secrets:
+      AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
+      AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
+      AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
     permissions:
       contents: read
       pull-requests: write
       security-events: write
-
-    steps:
-      - name: Check out repo
-        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
-        with:
-          ref: ${{ github.event.pull_request.head.sha }}
-
-      - name: Scan with Checkmarx
-        uses: checkmarx/ast-github-action@ef93013c95adc60160bc22060875e90800d3ecfc # 2.3.19
-        env:
-          INCREMENTAL:
-            "${{ contains(github.event_name, 'pull_request') && '--sast-incremental' || '' }}"
-        with:
-          project_name: ${{ github.repository }}
-          cx_tenant: ${{ secrets.CHECKMARX_TENANT }}
-          base_uri: https://ast.checkmarx.net/
-          cx_client_id: ${{ secrets.CHECKMARX_CLIENT_ID }}
-          cx_client_secret: ${{ secrets.CHECKMARX_SECRET }}
-          additional_params: |
-            --report-format sarif \
-            --filter "state=TO_VERIFY;PROPOSED_NOT_EXPLOITABLE;CONFIRMED;URGENT" \
-            --output-path . ${{ env.INCREMENTAL }}
-
-      - name: Upload Checkmarx results to GitHub
-        uses: github/codeql-action/upload-sarif@ce28f5bb42b7a9f2c824e633a3f6ee835bab6858 # v3.29.0
-        with:
-          sarif_file: cx_result.sarif
-          sha:
-            ${{ contains(github.event_name, 'pull_request') && github.event.pull_request.head.sha ||
-            github.sha }}
-          ref:
-            ${{ contains(github.event_name, 'pull_request') && format('refs/pull/{0}/head',
-            github.event.pull_request.number) || github.ref }}
+      id-token: write
 
   quality:
-    name: Quality scan
-    runs-on: ubuntu-22.04
+    name: Sonar
+    uses: bitwarden/gh-actions/.github/workflows/_sonar.yml@main
     needs: check-run
+    secrets:
+      AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
+      AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
+      AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
     permissions:
       contents: read
       pull-requests: write
-
-    steps:
-      - name: Check out repo
-        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
-        with:
-          fetch-depth: 0
-          ref: ${{  github.event.pull_request.head.sha }}
-
-      - name: Scan with SonarCloud
-        uses: sonarsource/sonarqube-scan-action@2500896589ef8f7247069a56136f8dc177c27ccf # v5.2.0
-        env:
-          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
-        with:
-          args: >
-            -Dsonar.organization=${{ github.repository_owner }}  -Dsonar.projectKey=${{
-            github.repository_owner }}_${{ github.event.repository.name }}
-            -Dsonar.pullrequest.key=${{ github.event.pull_request.number }}
+      id-token: write

custom-words.txt

@@ -61,6 +61,8 @@ recompositions
 refactorings
 roadmap
 roadmaps
+rollout
+rollouts
 rustup
 sandboxed
 SARIF
@@ -85,6 +87,7 @@ typechecks
 typesafe
 WCAG
 Xcodes.app
+xcworkspace
 xmldoc
 Yellowpages
 Yubico

docs/architecture/adr/0026-dotnet-dependency-injection-enhancements.md

@@ -1,6 +1,6 @@
 ---
 adr: "0026"
-status: "Proposed"
+status: "Accepted"
 date: 2025-05-30
 tags: [server]
 ---
@@ -120,7 +120,7 @@ as they are expected to always be included in the host -- those services are `IL
 
 ## Decision Outcome
 
-Chosen option: **Encourage usage**.
+Chosen option: **Enforce usage**.
 
 ### Positive Consequences
 
@@ -136,10 +136,10 @@ Chosen option: **Encourage usage**.
 
 ### Plan
 
-The plan to encourage the usage will be to update our C# style guide with the recommendation to
-begin using the `TryAdd` overloads (the document should explain when you want to use each one). The
-coding guidelines can be part of a newly-added document about general dependency injection
-guidelines for .NET here at Bitwarden.
+Enforcing usage through the `Microsoft.CodeAnalysis.BannedApiAnalyzers` NuGet package, we will tone
+down the diagnostic from the banned APIs to be warnings instead of errors. We will have the warning
+include custom diagnostic text to point to a new section in the C# style guide with the instructions
+on how to migrate.
 
 A one-time recorded learning session will also be hosted; the session will go over the new docs,
 show off migrating existing service registrations to using the `TryAdd` overloads, and host a QnA.

docs/contributing/code-style/csharp.md

@@ -130,6 +130,46 @@ For example you might have an assertion like `Debug.Assert(user.IsVerified)` in
 that only expects verified users. `Debug.Assert` works by using one of the aforementioned attributes
 `[DoesNotReturnIf(false)]` to make it work in case you want to implement your own assertion.
 
+## Dependency Injection
+
+### Use `TryAdd*` overloads on `IServiceCollection`
+
+Use [`TryAdd*` overloads][tryadd-overloads] as opposed to the less clear versions like
+`AddSingleton` and `AddKeyedTransient`. If you want to use your service using multi-inject then you
+should use the `TryAddEnumerable` method along with the static factory methods on
+`ServiceDescriptor` to register your service. This has the benefit of allowing your services to be
+registered many times during startup and not polluting the collection with duplicates that either
+won't ever be used or might accidentally be used if someone wants to inject multiple of that service
+type.
+
+Single inject scenario:
+
+```csharp
+// ❌ Don't do this
+services.AddSingleton<IMyService, DefaultMyService>();
+// ✅ Do this instead
+services.TryAddSingleton<IMyService, DefaultMyService>();
+```
+
+Multi-inject scenario:
+
+```csharp
+// ❌ Don't do this
+services.AddKeyedTransient<IMyService, FirstMyService>("first");
+// ✅ Do this instead
+services.TryAddEnumerable(ServiceDescriptor.KeyedTransient<IMyService, FirstMyService>("first"));
+```
+
+In multi-inject scenarios you are especially in danger of adding the same implementation multiple
+times and having them all be used.
+
+### Dependency Groups
+
+Consider creating [dependency groups][dependency-groups]. This gives other teams a nice interface to
+register your group when they depend on it. If you've used
+[`TryAdd`](#use-tryadd-overloads-on-iservicecollection) overloads it won't matter how many times
+your group of services is added to the collection.
+
 [null-forgiving]:
   https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-forgiving
 [null-state-attributes]:
@@ -138,3 +178,7 @@ that only expects verified users. `Debug.Assert` works by using one of the afore
   https://learn.microsoft.com/en-us/aspnet/core/mvc/models/validation?view=aspnetcore-9.0#non-nullable-reference-types-and-required-attribute
 [debug-assert]:
   https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debug.assert?view=net-9.0
+[tryadd-overloads]:
+  https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.extensions.servicecollectiondescriptorextensions?view=net-9.0-pp
+[dependency-groups]:
+  https://learn.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-9.0#register-groups-of-services-with-extension-methods

docs/contributing/database-migrations/_category_.yml

@@ -0,0 +1,2 @@
+label: "Database Migrations"
+position: 2

docs/contributing/database-migrations/edd.mdx

@@ -1,3 +1,7 @@
+---
+sidebar_position: 3
+---
+
 import Tabs from "@theme/Tabs";
 import TabItem from "@theme/TabItem";
 
@@ -130,6 +134,8 @@ actions.
 <Tabs>
 <TabItem value="first" label="Initial Migration" default>
 
+Located in `util/Migrator/DbScripts`.
+
 ```sql
 -- Add Column
 IF COL_LENGTH('[dbo].[Customer]', 'FirstName') IS NULL
@@ -177,6 +183,8 @@ END
 </TabItem>
 <TabItem value="data" label="Transition Migration">
 
+Located in `util/Migrator/DbScripts_transition`.
+
 ```sql
 UPDATE [dbo].Customer SET
     FirstName=FName
@@ -186,6 +194,8 @@ WHERE FirstName IS NULL
 </TabItem>
 <TabItem value="second" label="Finalization Migration">
 
+Located in `util/Migrator/DbScripts_finalization`.
+
 ```sql
 -- Remove Column
 IF COL_LENGTH('[dbo].[Customer]', 'FName') IS NOT NULL
@@ -239,7 +249,7 @@ There are some important constraints to the implementation of the process:
 
 The process to support all of these constraints is a complex one. Below is an image of a state
 machine that will hopefully help visualize the process and what it supports. It assumes that all
-database changes follow the standards that are laid out in [Migrations](./).
+database changes follow the standards that are laid out in [Migrations](./mssql.md).
 
 ---
 

docs/contributing/database-migrations/ef.md

@@ -0,0 +1,31 @@
+---
+sidebar_position: 2
+---
+
+# Entity Framework
+
+:::info
+
+For instructions on how to apply database migrations, please refer to the
+[Getting Started](../../getting-started/server/database/ef/index.mdx#updating-the-database)
+documentation.
+
+:::
+
+If you alter the database schema, you must create an EF migration script to ensure that EF databases
+keep pace with these changes. Developers must do this and include the migrations with their PR.
+
+To create these scripts, you must first update your data model in `Core/Entities` as desired. This
+will be used to generate the migrations for each of our EF targets. Additionally, for table changes
+it is strongly recommended to define or update an `IEntityTypeConfiguration<T>` to accurately
+represent any constraints needed on the data model.
+
+Once the model is updated, navigate to the `dev` directory in the `server` repo and execute the
+`ef_migrate.ps1` PowerShell command. You should provide a name for the migration as the only
+parameter:
+
+```bash
+pwsh ef_migrate.ps1 [NAME_OF_MIGRATION]
+```
+
+This will generate the migrations, which should then be included in your PR.