-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(auth-validator): [Auth/PM-22975] Client Version Validator #6588
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
Open
Patrick-Pimentel-Bitwarden
wants to merge
58
commits into
main
Choose a base branch
from
auth/pm-22975/client-version-validator
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 47 commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
1c4fd6c
feat(auth-validator): [PM-22975] Client Version Validator - initial iโฆ
Patrick-Pimentel-Bitwarden 22e9e5b
mend
Patrick-Pimentel-Bitwarden 1af2fba
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 47a26bb
fix(auth-validator): [PM-22975] Client Version Validator - Minor toucโฆ
Patrick-Pimentel-Bitwarden a82b31c
fix(auth-validator): [PM-22975] Client Version Validator - Fixing somโฆ
Patrick-Pimentel-Bitwarden 7d71ee2
fix(auth-validator): [PM-22975] Client Version Validator - Finished fโฆ
Patrick-Pimentel-Bitwarden 851f963
test(auth-validator): [PM-22975] Client Version Validator - Fixed tesโฆ
Patrick-Pimentel-Bitwarden 756ae5e
fix(auth-validator): [PM-22975] Client Version Validator - Fixed namiโฆ
Patrick-Pimentel-Bitwarden 7874ec7
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 91af02b
test(auth-validator): [PM-22975] Client Version Validator - Fixed tests.
Patrick-Pimentel-Bitwarden 59d9d7b
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 7897485
fix(auth-validator): [PM-22975] Client Version Validator - Renamed vaโฆ
Patrick-Pimentel-Bitwarden d0c5333
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden e46425d
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden d69842d
fix(auth-validator): [PM-22975] Client Version Validator - Added headโฆ
Patrick-Pimentel-Bitwarden 1681703
fix(auth-validator): [PM-22975] Client Version Validator - Trying to โฆ
Patrick-Pimentel-Bitwarden cb146fc
fix(auth-validator): [PM-22975] Client Version Validator - Fixed tests
Patrick-Pimentel-Bitwarden e74682b
Merge remote-tracking branch 'origin' into auth/pm-22975/client-versiโฆ
Patrick-Pimentel-Bitwarden 53e6509
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden b3b1b9b
fix(auth-validator): [PM-22975] Client Version Validator - misc changโฆ
Patrick-Pimentel-Bitwarden 8f89694
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 8b8694e
test(auth-validator): [PM-22975] Client Version Validator - WIP changes.
Patrick-Pimentel-Bitwarden ed89cf8
fix(auth-validator): [PM-22975] Client Version Validator - Made enougโฆ
Patrick-Pimentel-Bitwarden 6696104
fix(auth-validator): [PM-22975] Client Version Validator - Fixed moreโฆ
Patrick-Pimentel-Bitwarden aa4f8ab
test(auth-validator): [PM-22975] Client Version Validator - Fixed theโฆ
Patrick-Pimentel-Bitwarden 86bca81
fix(auth-validator): [PM-22975] Client Version Validator - Changed soโฆ
Patrick-Pimentel-Bitwarden c1bc10b
fix(auth-validator): [PM-22975] Client Version Validator - Removed unโฆ
Patrick-Pimentel-Bitwarden 753670d
fix(auth-validator): [PM-22975] Client Version Validator - Took in feโฆ
Patrick-Pimentel-Bitwarden 544965e
test(auth-validator): [PM-22975] Client Version Validator - Fixed test
Patrick-Pimentel-Bitwarden f719763
fix(auth-validator): [PM-22975] Client Version Validator - Took in teโฆ
Patrick-Pimentel-Bitwarden cff2f5d
fix(auth-validator): [PM-22975] Client Version Validator - Added moreโฆ
Patrick-Pimentel-Bitwarden 2ed458d
fix(auth-validator): [PM-22975] Client Version Validator - Removed onโฆ
Patrick-Pimentel-Bitwarden d706796
fix(auth-validator): [PM-22975] Client Version Validator - Updated vaโฆ
Patrick-Pimentel-Bitwarden 2264056
fix(auth-validator): [PM-22975] Client Version Validator - Updated wiโฆ
Patrick-Pimentel-Bitwarden 55bfb71
test(auth-validator): [PM-22975] Client Version Validator - Added encโฆ
Patrick-Pimentel-Bitwarden 36e7b1c
test(auth-validator): [PM-22975] Client Version Validator - Added stuโฆ
Patrick-Pimentel-Bitwarden 1f8be3b
docs(auth-validator): [PM-22975] Client Version Validator - Updated cโฆ
Patrick-Pimentel-Bitwarden 998aeb1
fix(auth-validator): [PM-22975] Client Version Validator - Not havingโฆ
Patrick-Pimentel-Bitwarden 27c9e4d
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden fb83df3
fix(auth-validator): [PM-22975] Client Version Validator - Minor toucโฆ
Patrick-Pimentel-Bitwarden 10cbfd8
fix(auth-validator): [PM-22975] Client Version Validator - Minor funcโฆ
Patrick-Pimentel-Bitwarden 79256d7
fix(auth-validator): [PM-22975] Client Version Validator - Minor chanโฆ
Patrick-Pimentel-Bitwarden 865e76f
fix(auth-validator): [PM-22975] Client Version Validator - Reorder ofโฆ
Patrick-Pimentel-Bitwarden 50298fb
fix(auth-validator): [PM-22975] Client Version Validator - Fixed testโฆ
Patrick-Pimentel-Bitwarden ad68312
fix(auth-validator): [PM-22975] Client Version Validator - Added tickโฆ
Patrick-Pimentel-Bitwarden 3f0d7d2
test(auth-validator): [PM-22975] Client Version Validator - added oneโฆ
Patrick-Pimentel-Bitwarden fc6a513
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 06224d9
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 6a5518a
fix(auth-validator): [PM-22975] Client Version Validator - Rename funโฆ
Patrick-Pimentel-Bitwarden fcd4b24
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 3736944
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 6ddd8c7
Merge branch 'main' into auth/pm-22975/client-version-validator
JaredSnider-Bitwarden c7ea4ea
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 3bd54e5
test(auth-validator): [PM-22975] Client Version Validator - Fixed tests.
Patrick-Pimentel-Bitwarden f44cd58
Merge remote-tracking branch 'origin' into auth/pm-22975/client-versiโฆ
Patrick-Pimentel-Bitwarden 9a1f91f
fix(auth-validator): [PM-22975] Client Version Validator - Fixed tests.
Patrick-Pimentel-Bitwarden a3f5a32
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden 19d43e9
Merge branch 'main' into auth/pm-22975/client-version-validator
Patrick-Pimentel-Bitwarden File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| ๏ปฟnamespace Bit.Core.KeyManagement; | ||
|
|
||
| public static class Constants | ||
| { | ||
| public static readonly Version MinimumClientVersionForV2Encryption = new("2025.11.0"); | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ๐ Hardcoded version requires code deployment to update - This hardcoded version requires a code deployment and release cycle to change. Recommendations:
Example documentation: /// <summary>
/// Minimum client version required for users with v2 encryption.
/// Update when v2 encryption support is added to new client releases.
/// This version (2025.11.0) corresponds to the first client release with v2 support.
/// See: https://bitwarden.atlassian.net/browse/PM-22975
/// </summary>
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JaredSnider-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| ๏ปฟusing Bit.Core.Enums; | ||
|
|
||
| namespace Bit.Core.KeyManagement.Utilities; | ||
|
|
||
| public static class EncryptionParsing | ||
| { | ||
| /// <summary> | ||
| /// Helper method to convert an encryption type string to an enum value. | ||
| /// </summary> | ||
| public static EncryptionType GetEncryptionType(string? encString) | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| ArgumentNullException.ThrowIfNull(encString); | ||
|
|
||
| var parts = encString.Split('.'); | ||
| if (parts.Length == 1) | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| throw new ArgumentException("Invalid encryption type string."); | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| if (byte.TryParse(parts[0], out var encryptionTypeNumber)) | ||
| { | ||
| if (Enum.IsDefined(typeof(EncryptionType), encryptionTypeNumber)) | ||
| { | ||
| return (EncryptionType)encryptionTypeNumber; | ||
| } | ||
| } | ||
| throw new ArgumentException("Invalid encryption type string."); | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
92 changes: 92 additions & 0 deletions
92
src/Identity/IdentityServer/RequestValidators/ClientVersionValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| ๏ปฟusing Bit.Core.Context; | ||
| using Bit.Core.Entities; | ||
| using Bit.Core.KeyManagement; | ||
| using Bit.Core.Models.Api; | ||
| using Duende.IdentityServer.Validation; | ||
|
|
||
| namespace Bit.Identity.IdentityServer.RequestValidators; | ||
|
|
||
| public interface IClientVersionValidator | ||
| { | ||
| bool ValidateAsync(User user, CustomValidatorRequestContext requestContext); | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /// <summary> | ||
| /// This validator will use the Client Version on a request, which currently maps | ||
| /// to the "Bitwarden-Client-Version" header, to determine if a user meets minimum | ||
| /// required client version for issuing tokens on an old client. This is done to | ||
| /// incentivize users to get on an updated client when their password encryption | ||
| /// method has already been updated. | ||
| /// | ||
| /// If the header is omitted, then the validator returns that this request is invalid. | ||
| /// </summary> | ||
| public class ClientVersionValidator( | ||
| ICurrentContext currentContext) | ||
| : IClientVersionValidator | ||
| { | ||
| private const string _upgradeMessage = "Please update your app to continue using Bitwarden"; | ||
| private const string _noUserMessage = "No user found while trying to validate client version"; | ||
| private const string _versionHeaderMissing = "No client version header found, required to prevent encryption errors. Please confirm your client is supplying the header: \"Bitwarden-Client-Version\""; | ||
|
|
||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public bool ValidateAsync(User? user, CustomValidatorRequestContext requestContext) | ||
| { | ||
| // Do this nullish check because the base request validator currently is not | ||
| // strict null checking. Once that gets fixed then we can see about making | ||
| // the user not nullish checked. If they are null then the validator should fail. | ||
| if (user == null) | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| FillRequestContextWithErrorData(requestContext, "no_user", _noUserMessage); | ||
| return false; | ||
| } | ||
|
|
||
| Version? clientVersion = currentContext.ClientVersion; | ||
|
|
||
| // Deny access if the client version headers are missing. | ||
| // We want to establish a strict contract with clients that if they omit this header, | ||
| // then the server cannot guarantee that a client won't do harm to a user's data | ||
| // with stale encryption architecture. | ||
| if (clientVersion == null) | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| FillRequestContextWithErrorData(requestContext, "version_header_missing", _versionHeaderMissing); | ||
| return false; | ||
| } | ||
|
|
||
| // Determine the minimum version client that a user needs. If no V2 encryption detected then | ||
| // no validation needs to occur, which is why min version number can be null. | ||
| Version? minVersion = user.HasV2Encryption() ? Constants.MinimumClientVersionForV2Encryption : null; | ||
|
|
||
| // If min version is null then we know that the user had an encryption | ||
| // configuration that doesn't require a minimum version. Allowing through. | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (minVersion == null) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| if (clientVersion < minVersion) | ||
| { | ||
| FillRequestContextWithErrorData(requestContext, "invalid_client_version", _upgradeMessage); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private void FillRequestContextWithErrorData( | ||
| CustomValidatorRequestContext requestContext, | ||
| string errorId, | ||
| string errorMessage) | ||
| { | ||
| requestContext.ValidationErrorResult = new ValidationResult | ||
| { | ||
| Error = errorId, | ||
| ErrorDescription = errorMessage, | ||
| IsError = true | ||
| }; | ||
| requestContext.CustomResponse = new Dictionary<string, object> | ||
| { | ||
| { "ErrorModel", new ErrorResponseModel(errorMessage) } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.