[PM-40216] feat: Data-protect OrganizationInviteLink.Code at rest#7969
[PM-40216] feat: Data-protect OrganizationInviteLink.Code at rest#7969r-tome wants to merge 7 commits into
Conversation
Stores invite link Code as IDataProtector-encrypted NVARCHAR(300) with a "P|" prefix, replacing the plaintext UNIQUEIDENTIFIER. All anonymous endpoints now require OrganizationId + Code; lookup by Code is replaced with lookup-by-org + constant-time FixedTimeEquals comparison, eliminating the Code as a DB lookup key and closing the DB-read bearer-secret exposure. Includes SQL Server migration (coordinated deploy), EF Core migrations for MySQL/PostgreSQL/SQLite, SSDT schema update, and the InviteLinkCodeValidator helper for constant-time comparisons.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the data-protection change for No findings at or above the confidence threshold. The two existing review threads (lowercase normalization on EF providers, EDD/no-rollback concern on the coordinated-deploy migration) were satisfactorily resolved by the author: the feature is unreleased with zero existing records, so the intentionally non-backwards-compatible column-type change and dropped Code Review DetailsNo findings at or above the confidence threshold. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| migrationBuilder.DropIndex( | ||
| name: "IX_OrganizationInviteLink_Code", | ||
| table: "OrganizationInviteLink"); | ||
| } |
There was a problem hiding this comment.
❓ QUESTION: Are pre-existing invite-link codes normalized to lowercase on SQLite/MySQL/PostgreSQL?
Details
The MSSQL migration deliberately runs UPDATE ... SET [Code] = LOWER([Code]) with the comment "SQL Server casts UNIQUEIDENTIFIER to uppercase NVARCHAR; LOWER() normalizes for FixedTimeEquals". This is because InviteLinkCodeValidator.CodesMatch uses CryptographicOperations.FixedTimeEquals on raw UTF-8 bytes — a case-sensitive comparison — while the incoming request.Code.ToString() is always lowercase (Guid.ToString() in .NET is lowercase).
This SQLite migration (and the MySQL/Postgres ones) only drops the index / alters the column type; none of them lowercase existing rows. For legacy invite links created before this PR, Code was stored via EF's Guid-to-TEXT conversion — the same conversion used for Id/OrganizationId, which EF renders in uppercase in SQLite.
If SQLite stored those legacy codes uppercase, they will no longer match the lowercase incoming code after migration, silently breaking every pre-existing invite link on self-hosted SQLite deployments (Postgres uuid→text and MySQL char(36) are lowercase, so likely unaffected).
Can you confirm the stored casing on each EF provider, and add a normalization step for any provider that stores uppercase — mirroring the MSSQL LOWER() — so existing links keep working?
There was a problem hiding this comment.
There are no pre-existing invite-link codes. This feature has not been released yet.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7969 +/- ##
==========================================
+ Coverage 62.05% 66.61% +4.56%
==========================================
Files 2277 2277
Lines 99452 99556 +104
Branches 8983 8990 +7
==========================================
+ Hits 61718 66323 +4605
+ Misses 35561 30961 -4600
- Partials 2173 2272 +99 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
r-tome
changed the title
…-invite-link-codes # Conflicts: # test/Api.Test/AdminConsole/Controllers/OrganizationInviteLinksControllerTests.cs # test/Core.Test/AdminConsole/OrganizationFeatures/InviteLinks/GetOrganizationInviteLinkStatusQueryTests.cs
…ation CI's migration order check failed because main gained 2026-07-14_00_AddCreationDateToOrganizationUserUserDetailsView.sql after this branch's migration was authored.
…eValidator Delegates to the codebase's existing constant-time comparison helper instead of duplicating the raw CryptographicOperations call.
…onInviteLink Replaces the standalone InviteLinkCodeValidator with an instance method on OrganizationInviteLink, matching the AuthRequest.IsValidForAuthentication pattern of entities comparing their own fields against external input.
The table has zero rows in every environment, so there's no existing data to normalize before the column type change.
There was a problem hiding this comment.
No records exist whatsoever so no EDD rules are broken. This was confirmed here #7896 (review)
…-invite-link-codes # Conflicts: # test/Core.Test/AdminConsole/OrganizationFeatures/InviteLinks/GetOrganizationInviteLinkPoliciesQueryTests.cs
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-40216
📔 Objective
The invite link
Codeis a bearer secret — anyone with DB read access (breach, backup dump, self-hosted operator) could obtain every org's join capability and walk a compliant email to Accepted. This PR data-protectsCodeat rest viaIDataProtector, matching the existing pattern inUserRepositoryandSendRepository.Key changes:
OrganizationInviteLink.Codechanges fromGuidtostring(wire type staysGuid); stored as"P|" + protector.Protect(code)in a newNVARCHAR(300)columnGetByCodeAsyncis removed; all anonymous endpoints now require bothOrganizationIdandCode— fetch by org, then validateCodeviaCryptographicOperations.FixedTimeEquals(constant-time, no oracle)Codedropped (encrypted values are non-deterministic)InviteLinkCodeValidatorstatic helper centralises the constant-time comparison/#/join/{orgId}/{code}?key=...(clients PR companion)Security properties after this change: