Skip to content

Commit 0d66de0

Browse files
mcamiraultmcamirault
committed
Merge branch 'tools/pm-31884/send-access-controls-policy' into tools/pm-31929/send-deletion-days-policy
2 parents 761f257 + 0732302 commit 0d66de0

575 files changed

Lines changed: 62219 additions & 5926 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/skills/implementing-dapper-queries/SKILL.md‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
---
2+
name: implementing-dapper-queries
3+
description: Implementing Dapper repository methods and stored procedures for MSSQL at Bitwarden. Use when creating or modifying Dapper repositories, writing stored procedures, or working with MSSQL-specific data access in the server repo. Also use when writing MSSQL migration scripts under `util/Migrator/DbScripts/` or touching SSDT schema under `src/Sql/dbo/`.
4+
---
5+
6+
## Repository Pattern
7+
8+
All Dapper implementations live in `src/Infrastructure/Dapper/Repositories/`. Each repository class implements an interface from `src/Core/` and uses stored procedures for all database operations. The repository method is intentionally thin — it maps C# parameters to SQL parameters and maps result sets back to domain objects.
9+
10+
### Stored procedures over inline SQL
11+
12+
The default pattern is stored procedures for all Dapper database operations. Some exceptions exist where inline SQL is used — these are provided automatically by the repository base class and parent patterns, not written ad-hoc in individual repository methods.
13+
14+
## Workflow
15+
16+
1. **Define/update the stored procedure** in `src/Sql/dbo/Stored Procedures/` — use plain `CREATE PROCEDURE` (SSDT syntax)
17+
2. **Create a migration script** in `util/Migrator/DbScripts/` that deploys it — use `CREATE OR ALTER PROCEDURE` (idempotent)
18+
3. **Implement the repository method** in `src/Infrastructure/Dapper/Repositories/` using `DapperServiceProvider` to call the procedure
19+
4. **Write integration tests** using `[DatabaseData]` attribute
20+
21+
The stored procedure is the source of truth for MSSQL query behavior. The Dapper repository method is thin — it maps parameters and results.
22+
23+
### Stored procedure naming convention
24+
25+
Procedures follow `{Entity}_{Action}` pattern: `User_Create`, `Cipher_ReadManyByUserId`, `Organization_DeleteById`. Tooling and code generation rely on this convention to map repository methods to their procedures.
26+
27+
## Key Decisions That Trip Up AI Assistants
28+
29+
### `CREATE OR ALTER` vs `CREATE PROCEDURE` — depends on file location
30+
31+
Bitwarden maintains two copies of every stored procedure in different contexts with different toolchain constraints:
32+
33+
| Context | Location | Required syntax |
34+
| ---------------------- | -------------------------------- | --------------------------- |
35+
| **SSDT schema source** | `src/Sql/dbo/Stored Procedures/` | `CREATE PROCEDURE` (plain) |
36+
| **Migration script** | `util/Migrator/DbScripts/` | `CREATE OR ALTER PROCEDURE` |
37+
38+
**Why they differ:**
39+
40+
- **SSDT projects** do not support `CREATE OR ALTER` — using it produces build errors. SSDT manages object lifecycle through its own deployment model, so each source file must contain a bare `CREATE PROCEDURE`.
41+
- **Migration scripts** must be idempotent because they may be re-run. `CREATE OR ALTER` works whether the procedure exists or not. Never use bare `CREATE PROCEDURE` in a migration.
42+
43+
### SSDT table files require `GO` batch separators
44+
45+
In `src/Sql/dbo/Tables/`, SSDT requires a `GO` batch separator between `CREATE TABLE` and any subsequent `CREATE INDEX` or `CREATE NONCLUSTERED INDEX` statements.
46+
47+
```sql
48+
-- CORRECT — GO separates DDL statements for SSDT
49+
CREATE TABLE [dbo].[Example] (
50+
[Id] UNIQUEIDENTIFIER NOT NULL,
51+
[Name] NVARCHAR(256) NOT NULL,
52+
CONSTRAINT [PK_Example] PRIMARY KEY CLUSTERED ([Id] ASC)
53+
)
54+
GO
55+
56+
CREATE NONCLUSTERED INDEX [IX_Example_Name]
57+
ON [dbo].[Example] ([Name] ASC)
58+
GO
59+
```
60+
61+
### New parameters must be nullable with defaults
62+
63+
When adding parameters to existing stored procedures, always use `@NewParam DATATYPE = NULL`. Existing callers don't pass the new parameter — without a default, they break.
64+
65+
### NOT NULL columns: use inline defaults, not ALTER-UPDATE-ALTER
66+
67+
Adding a NOT NULL column by first adding it nullable, updating all rows, then altering to NOT NULL causes a full table scan. Instead, use `ADD [Column] INT NOT NULL CONSTRAINT DF_Table_Column DEFAULT 0` — this is a metadata-only operation in SQL Server. **This is the single most common mistake AI assistants make with Bitwarden migrations.**
68+
69+
### Never create indexes on large tables in migration scripts
70+
71+
Creating indexes on `dbo.Cipher`, `dbo.OrganizationUser`, or other large tables in migration scripts can cause outages. Never specify `ONLINE = ON` in scripts — production handles this automatically, and the option fails on unsupported SQL Server editions. Large index operations belong in `DbScripts_manual`.
72+
73+
### Use defaults only for numeric types
74+
75+
Use defaults for `BIT`, `TINYINT`, `INT`, `BIGINT`. Never use defaults for `VARCHAR`, `NVARCHAR`, or MAX types. SQL Server handles these differently and defaults on strings create unexpected behavior with EF Core migrations.
76+
77+
### Views require metadata refresh
78+
79+
After modifying a table, any views that reference it have stale metadata. Call `sp_refreshview` on affected views. After altering views, call `sp_refreshsqlmodule` on dependent procedures. This is the most frequently forgotten step.
80+
81+
### GUID columns use `UNIQUEIDENTIFIER`
82+
83+
All entity IDs are `UNIQUEIDENTIFIER` populated by `CoreHelpers.GenerateComb()` in application code, not by SQL Server. Never use `NEWID()` or `NEWSEQUENTIALID()` in stored procedures.
84+
85+
## EF Parity Requirement
86+
87+
Every stored procedure's behavior must be exactly replicated in the EF Core implementation. When writing a new stored procedure, think about how the EF implementation will reproduce the same filtering, ordering, and side effects. If a stored procedure does something complex (e.g., conditional updates, multi-table operations), document the expected behavior clearly so the EF implementation can match it.
88+
89+
## Critical Rules
90+
91+
These are the most frequently violated conventions. Claude cannot fetch the linked docs at runtime, so these are inlined here:
92+
93+
- **`SET NOCOUNT ON`** at the start of every stored procedure
94+
- **Parameter naming:** `@ParamName` in PascalCase, matching C# property names
95+
- **Migration scripts must be idempotent** — use `CREATE OR ALTER` in `util/Migrator/DbScripts/`; use plain `CREATE PROCEDURE` in SSDT source (`src/Sql/dbo/`)
96+
- **Constraint naming:** `PK_TableName`, `FK_Child_Parent`, `IX_Table_Column`, `DF_Table_Column`
97+
- **Stored procedure file naming:** one procedure per file, named `{Entity}_{Action}.sql`
98+
99+
## Examples
100+
101+
### Stored procedure creation — SSDT source vs migration script
102+
103+
```sql
104+
-- SSDT source file: src/Sql/dbo/Stored Procedures/User_ReadById.sql
105+
-- Use plain CREATE PROCEDURE (SSDT does not support CREATE OR ALTER)
106+
CREATE PROCEDURE [dbo].[User_ReadById]
107+
@Id UNIQUEIDENTIFIER
108+
AS
109+
BEGIN
110+
SET NOCOUNT ON
111+
SELECT * FROM [dbo].[User] WHERE [Id] = @Id
112+
END
113+
```
114+
115+
```sql
116+
-- Migration script: util/Migrator/DbScripts/YYYY-MM-DD_00_AddUser_ReadById.sql
117+
-- Use CREATE OR ALTER for idempotency
118+
CREATE OR ALTER PROCEDURE [dbo].[User_ReadById]
119+
@Id UNIQUEIDENTIFIER
120+
AS
121+
BEGIN
122+
SET NOCOUNT ON
123+
SELECT * FROM [dbo].[User] WHERE [Id] = @Id
124+
END
125+
```
126+
127+
### Adding a NOT NULL column
128+
129+
```sql
130+
-- CORRECT — metadata-only operation, no table scan
131+
ALTER TABLE [dbo].[Organization]
132+
ADD [UseCustomPermissions] BIT NOT NULL CONSTRAINT DF_Organization_UseCustomPermissions DEFAULT 0
133+
134+
-- WRONG — causes full table scan on large tables
135+
ALTER TABLE [dbo].[Organization] ADD [UseCustomPermissions] BIT NULL
136+
UPDATE [dbo].[Organization] SET [UseCustomPermissions] = 0
137+
ALTER TABLE [dbo].[Organization] ALTER COLUMN [UseCustomPermissions] BIT NOT NULL
138+
```
139+
140+
### Adding parameters to existing procedures
141+
142+
```sql
143+
-- CORRECT — existing callers won't break
144+
CREATE OR ALTER PROCEDURE [dbo].[Cipher_Create]
145+
@Id UNIQUEIDENTIFIER,
146+
@NewField NVARCHAR(MAX) = NULL -- default protects existing callers
147+
148+
-- WRONG — breaks all existing callers immediately
149+
CREATE OR ALTER PROCEDURE [dbo].[Cipher_Create]
150+
@Id UNIQUEIDENTIFIER,
151+
@NewField NVARCHAR(MAX) -- no default = required parameter
152+
```
153+
154+
## Further Reading
155+
156+
- [SQL code style](https://contributing.bitwarden.com/contributing/code-style/sql/)
157+
- [Database migrations (MSSQL)](https://contributing.bitwarden.com/contributing/database-migrations/mssql)

‎.claude/skills/implementing-ef-core/SKILL.md‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
---
2+
name: implementing-ef-core
3+
description: Implementing Entity Framework Core repositories and migrations for PostgreSQL, MySQL, and SQLite at Bitwarden. Use when creating or modifying EF repositories, generating EF migrations, or working with non-MSSQL data access in the server repo. Also use when editing `EntityTypeConfiguration<T>` classes or debugging provider-specific LINQ translation issues.
4+
---
5+
6+
## Repository Pattern
7+
8+
EF implementations live in `src/Infrastructure/EntityFramework/Repositories/`. Each class implements the same interface as its Dapper counterpart. The EF repository uses `DbContext` and LINQ queries instead of stored procedures, but must produce identical behavior.
9+
10+
### Why behavior must match stored procedures exactly
11+
12+
Bitwarden self-hosted runs on the customer's choice of database. If `CipherRepository.GetManyByUserId()` returns results in a different order on PostgreSQL than the stored procedure returns on MSSQL, or filters differently, or handles nulls differently — that's a bug. Users switching databases or comparing behavior across environments will see inconsistencies.
13+
14+
The `[DatabaseData]` integration test attribute runs the same test against all configured databases. This is the primary safety net for parity.
15+
16+
### Cross-database considerations
17+
18+
EF Core's LINQ-to-SQL translation varies by provider. Patterns that work on one database may fail on another:
19+
20+
- **PostgreSQL** is stricter about types — operations like `Min()` on booleans or implicit string/int conversions that MySQL allows will throw
21+
- **SQLite** has limited ALTER TABLE support — some migrations that work elsewhere fail on SQLite
22+
- **Case sensitivity** depends on database collation, not on C# code — don't assume case-insensitive string comparison
23+
24+
The pragmatic approach: write clean LINQ, run `[DatabaseData]` tests, and fix provider-specific failures as they surface rather than trying to predict every edge case.
25+
26+
## Migration Generation
27+
28+
### Workflow
29+
30+
Run `pwsh ef_migrate.ps1 <MigrationName>` to generate migrations for all EF targets simultaneously. This creates migration files for each provider (PostgreSQL, MySQL, SQLite).
31+
32+
### Why the migration name matters
33+
34+
The EF migration class name must exactly match the MSSQL migration name portion (from the `YYYY-MM-DD_##_MigrationName.sql` filename). This convention keeps migration history aligned across ORMs and makes it easy to trace which EF migration corresponds to which SQL script.
35+
36+
### Always review generated migrations
37+
38+
EF's migration generator makes mechanical decisions that aren't always optimal:
39+
40+
- It may drop and recreate indexes instead of renaming them
41+
- It may generate unnecessary column modifications when model annotations change
42+
- It doesn't know about Bitwarden's large table concerns (never add indexes to `Cipher`, `OrganizationUser` etc. without careful review)
43+
44+
Review the generated `Up()` and `Down()` methods to ensure they align with the stored procedure migration's intent.
45+
46+
## Key Decisions That Trip Up AI Assistants
47+
48+
### Don't add navigation properties casually
49+
50+
EF navigation properties (e.g., `public virtual Organization Organization { get; set; }`) affect query generation and lazy loading behavior. Only add them when the stored procedure equivalent also joins those tables. Unnecessary navigation properties cause N+1 queries that don't match the stored procedure's behavior.
51+
52+
### DbContext configuration lives in `EntityTypeConfiguration` classes
53+
54+
Don't configure entities inline in `OnModelCreating`. Each entity has a configuration class that defines table mapping, relationships, and constraints. This keeps the DbContext clean and each entity's configuration self-contained.
55+
56+
### Respect the same GUID generation strategy
57+
58+
Entity IDs are generated in application code via `CoreHelpers.GenerateComb()`, not by the database. Don't configure `ValueGeneratedOnAdd()` or database-generated defaults for ID columns in EF configuration.
59+
60+
## Critical Rules
61+
62+
These are the most frequently violated conventions. Claude cannot fetch the linked docs at runtime, so these are inlined here:
63+
64+
- **One `EntityTypeConfiguration<T>` class per entity** — never configure inline in `OnModelCreating`
65+
- **Migration name must match MSSQL migration name** from `YYYY-MM-DD_##_MigrationName.sql`
66+
- **Run `pwsh ef_migrate.ps1 <Name>`** to generate migrations for all providers simultaneously
67+
- **Review `Up()` and `Down()` methods** in every generated migration before committing
68+
- **No `ValueGeneratedOnAdd()` on ID columns** — IDs come from `CoreHelpers.GenerateComb()` in app code
69+
70+
## Examples
71+
72+
### GUID configuration
73+
74+
```csharp
75+
// CORRECT — ID generated in application code
76+
public void Configure(EntityTypeBuilder<Cipher> builder)
77+
{
78+
builder.HasKey(c => c.Id);
79+
// No ValueGeneratedOnAdd — CoreHelpers.GenerateComb() handles this
80+
}
81+
82+
// WRONG — lets database generate IDs, breaks MSSQL parity
83+
public void Configure(EntityTypeBuilder<Cipher> builder)
84+
{
85+
builder.HasKey(c => c.Id);
86+
builder.Property(c => c.Id).ValueGeneratedOnAdd();
87+
}
88+
```
89+
90+
### Navigation properties
91+
92+
```csharp
93+
// CORRECT — only add when the SP also joins this table
94+
public class Cipher
95+
{
96+
public Guid Id { get; set; }
97+
public Guid OrganizationId { get; set; }
98+
// No navigation property — the SP doesn't JOIN Organization
99+
}
100+
101+
// WRONG — causes N+1 queries that don't match SP behavior
102+
public class Cipher
103+
{
104+
public Guid Id { get; set; }
105+
public Guid OrganizationId { get; set; }
106+
public virtual Organization Organization { get; set; }
107+
}
108+
```
109+
110+
## Further Reading
111+
112+
- [Database migrations (EF)](https://contributing.bitwarden.com/contributing/database-migrations/ef)
113+
- [SQL code style](https://contributing.bitwarden.com/contributing/code-style/sql/)

‎.claude/skills/writing-database-queries/SKILL.md‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
---
2+
name: writing-database-queries
3+
description: Bitwarden database architecture, migrations, and dual-ORM strategy. Use when working with `.sql` files, stored procedures, EF migrations, or database schema changes. Also use when deciding whether a change needs both Dapper and EF Core implementations, or whether a breaking stored-procedure change requires `_V2` versioning.
4+
---
5+
6+
## Dual-ORM Architecture
7+
8+
Bitwarden maintains two data access implementations, split by database provider:
9+
10+
- **MSSQL:** Dapper with stored procedures
11+
- **PostgreSQL, MySQL, SQLite:** Entity Framework Core
12+
13+
These implementations are **mutually exclusive at runtime** — SQL Server uses only Dapper, while the other providers use only EF Core. Both implementations conform to the same repository interfaces.
14+
15+
- When **adding new repository functionality**, implement it in **both** Dapper and EF Core (unless the feature is explicitly EF-only).
16+
- When **modifying an existing stored procedure** in a backwards-compatible way (for example, adding a new parameter with a default), **EF Core changes are not required**.
17+
- Some commercial features (for example, **Secrets Manager**) are **EF Core only**.
18+
19+
## Evolutionary Database Design (EDD)
20+
21+
Bitwarden Cloud uses a **no-rollback** approach to database deployments. The key implication: **server deployments can be rolled back, but database migrations cannot**, so migrations must be designed to avoid being a source of downtime.
22+
23+
All MSSQL migrations live in `util/Migrator/DbScripts/` and execute in chronological order based on the migration filename (`YYYY-MM-DD_##_Description.sql`).
24+
25+
> Note: You may see `util/Migrator/DbScripts_transition/` and `util/Migrator/DbScripts_finalization/` folders. These are not currently used; ignore them for now.
26+
27+
Simple additive changes (new nullable column, new table, new stored procedure) typically require only a single migration script in `util/Migrator/DbScripts/`.
28+
29+
### Stored procedure compatibility
30+
31+
Stored procedure changes fall into two categories:
32+
33+
- **Non-breaking (DEFAULT parameters):** Adding a parameter with a default value (e.g., `@NewParam BIT = NULL`) is backwards-compatible. Existing callers keep working; no `_V2` is needed.
34+
- **Breaking (`_V2` versioning):** Required when result-set structure changes, calling patterns change (e.g., single result → multiple result sets), required parameters are added without defaults, or query semantics differ. Implement this by creating `ProcedureName_V2` while retaining the original procedure for backwards compatibility.
35+
36+
Table-level breaking changes (removing columns, changing types) typically cascade into stored procedure changes and often require the `_V2` pattern.
37+
38+
**Always defer to the developer on migration strategy.** The approach is complex and context-dependent. When a database change is needed, write the migration script and ask the developer whether `_V2` versioning or additional steps are required.
39+
40+
## Key locations
41+
42+
- `src/Sql/dbo` — Master schema source of truth
43+
- `util/Migrator/DbScripts` — All migrations (single folder, chronological)
44+
45+
## ORM-Specific Implementation
46+
47+
When implementing Dapper repository methods, stored procedures, or MSSQL migration scripts, activate the `implementing-dapper-queries` skill.
48+
49+
When implementing EF Core repositories, generating EF migrations, or working with PostgreSQL/MySQL/SQLite, activate the `implementing-ef-core` skill.
50+
51+
## Critical Rules
52+
53+
These are the most frequently violated conventions. Claude cannot fetch the linked docs at runtime, so these are inlined here:
54+
55+
- **Migration file naming:** `YYYY-MM-DD_##_Description.sql` (e.g., `2025-06-15_00_AddVaultColumn.sql`)
56+
- **All schema objects use `dbo` schema** — never create objects in other schemas
57+
- **Constraint naming:** `PK_TableName` (primary key), `FK_Child_Parent` (foreign key), `IX_Table_Column` (index), `DF_Table_Column` (default)
58+
- **Idempotent scripts:** Use `IF NOT EXISTS` / `IF COL_LENGTH(...)` guards before schema changes in migration scripts
59+
- **New repository functionality requires both Dapper and EF Core implementations** — unless the feature is explicitly EF-only or the change is a backwards-compatible stored procedure modification
60+
- **Integration tests use `[DatabaseData]` attribute** — this runs the test against all configured database providers
61+
62+
## Further Reading
63+
64+
- [SQL code style](https://contributing.bitwarden.com/contributing/code-style/sql/)
65+
- [Database migrations (MSSQL)](https://contributing.bitwarden.com/contributing/database-migrations/mssql)
66+
- [Database migrations (EF)](https://contributing.bitwarden.com/contributing/database-migrations/ef)
67+
- [Evolutionary Database Design](https://contributing.bitwarden.com/contributing/database-migrations/edd)

0 commit comments

Comments
 (0)