-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(register): [PM-27084] Account Register Uses New Data Types #6715
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
base: main
Are you sure you want to change the base?
Changes from all commits
6c7daa6
1535fa3
9000474
d7ddf2e
08f3acd
f47dac9
bb2ba14
7cb55eb
b33e1ac
c21d7b4
aa8e8cc
5b2edef
57f69ad
4f81d75
fac1d4b
2d9786e
cc895d0
4475649
a705ea4
227f3a0
fb54d21
b0cce70
af2f704
015d2ed
67ff0da
fc507a4
05d8cc5
e531ab1
28640d0
260b289
06bf7b8
c255e39
3aa0e4c
2111df7
65c0ac9
a5890d2
b5dadcd
9e43ca2
93c9631
f73904d
f92d7d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| ๏ปฟ#nullable enable | ||
| using Bit.Core.Entities; | ||
| ๏ปฟusing Bit.Core.Entities; | ||
| using Bit.Core.Enums; | ||
| using Bit.Core.KeyManagement.Models.Api.Request; | ||
| using Bit.Core.Utilities; | ||
|
|
||
| namespace Bit.Core.Auth.Models.Api.Request.Accounts; | ||
|
|
@@ -21,19 +21,32 @@ public class RegisterFinishRequestModel : IValidatableObject | |
| public required string Email { get; set; } | ||
| public string? EmailVerificationToken { get; set; } | ||
|
|
||
| public MasterPasswordAuthenticationDataRequestModel? MasterPasswordAuthentication { get; set; } | ||
| public MasterPasswordUnlockDataRequestModel? MasterPasswordUnlock { get; set; } | ||
|
|
||
| // PM-28143 - Remove property below (made optional during migration to MasterPasswordUnlockData) | ||
| [StringLength(1000)] | ||
| public required string MasterPasswordHash { get; set; } | ||
| // Made optional but there will still be a thrown error if it does not exist either here or | ||
| // in the MasterPasswordAuthenticationData. | ||
| public string? MasterPasswordHash { get; set; } | ||
|
|
||
| [StringLength(50)] | ||
| public string? MasterPasswordHint { get; set; } | ||
|
|
||
| public required string UserSymmetricKey { get; set; } | ||
| // PM-28143 - Remove property below (made optional during migration to MasterPasswordUnlockData) | ||
| // Made optional but there will still be a thrown error if it does not exist either here or | ||
| // in the MasterPasswordAuthenticationData. | ||
| public string? UserSymmetricKey { get; set; } | ||
|
|
||
| public required KeysRequestModel UserAsymmetricKeys { get; set; } | ||
|
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. โน๏ธ This is outside of the scope of this PR, but fyi, for account registration V2, instead of
Contributor
Author
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. Thank you for sharing that knowledge, helps me understand future changes. |
||
|
|
||
| public required KdfType Kdf { get; set; } | ||
| public required int KdfIterations { get; set; } | ||
| // PM-28143 - Remove line below (made optional during migration to MasterPasswordUnlockData) | ||
| public KdfType? Kdf { get; set; } | ||
| // PM-28143 - Remove line below (made optional during migration to MasterPasswordUnlockData) | ||
| public int? KdfIterations { get; set; } | ||
| // PM-28143 - Remove line below | ||
| public int? KdfMemory { get; set; } | ||
| // PM-28143 - Remove line below | ||
| public int? KdfParallelism { get; set; } | ||
|
|
||
| public Guid? OrganizationUserId { get; set; } | ||
|
|
@@ -54,11 +67,14 @@ public User ToUser() | |
| { | ||
| Email = Email, | ||
| MasterPasswordHint = MasterPasswordHint, | ||
| Kdf = Kdf, | ||
| KdfIterations = KdfIterations, | ||
| KdfMemory = KdfMemory, | ||
| KdfParallelism = KdfParallelism, | ||
| Key = UserSymmetricKey, | ||
| Kdf = (KdfType)(MasterPasswordUnlock?.Kdf.KdfType ?? Kdf)!, | ||
| KdfIterations = (int)(MasterPasswordUnlock?.Kdf.Iterations ?? KdfIterations)!, | ||
| // KdfMemory and KdfParallelism are optional (only used for Argon2id) | ||
| KdfMemory = MasterPasswordUnlock?.Kdf.Memory ?? KdfMemory, | ||
| KdfParallelism = MasterPasswordUnlock?.Kdf.Parallelism ?? KdfParallelism, | ||
| // PM-28827 To be added when MasterPasswordSalt is added to the user column | ||
| // MasterPasswordSalt = MasterPasswordUnlock?.Salt ?? Email.ToLower().Trim(), | ||
| Key = MasterPasswordUnlock?.MasterKeyWrappedUserKey ?? UserSymmetricKey | ||
| }; | ||
|
|
||
| UserAsymmetricKeys.ToUser(user); | ||
|
|
@@ -72,29 +88,182 @@ public RegisterFinishTokenType GetTokenType() | |
| { | ||
| return RegisterFinishTokenType.EmailVerification; | ||
| } | ||
| if (!string.IsNullOrEmpty(OrgInviteToken) && OrganizationUserId.HasValue) | ||
| if (!string.IsNullOrEmpty(OrgInviteToken) | ||
| && OrganizationUserId.HasValue | ||
| && OrganizationUserId.Value != Guid.Empty) | ||
| { | ||
| return RegisterFinishTokenType.OrganizationInvite; | ||
| } | ||
| if (!string.IsNullOrWhiteSpace(OrgSponsoredFreeFamilyPlanToken)) | ||
| { | ||
| return RegisterFinishTokenType.OrgSponsoredFreeFamilyPlan; | ||
| } | ||
| if (!string.IsNullOrWhiteSpace(AcceptEmergencyAccessInviteToken) && AcceptEmergencyAccessId.HasValue) | ||
| if (!string.IsNullOrWhiteSpace(AcceptEmergencyAccessInviteToken) | ||
| && AcceptEmergencyAccessId.HasValue | ||
| && AcceptEmergencyAccessId.Value != Guid.Empty) | ||
| { | ||
| return RegisterFinishTokenType.EmergencyAccessInvite; | ||
| } | ||
| if (!string.IsNullOrWhiteSpace(ProviderInviteToken) && ProviderUserId.HasValue) | ||
| if (!string.IsNullOrWhiteSpace(ProviderInviteToken) | ||
| && ProviderUserId.HasValue | ||
| && ProviderUserId.Value != Guid.Empty) | ||
| { | ||
| return RegisterFinishTokenType.ProviderInvite; | ||
| } | ||
|
|
||
| throw new InvalidOperationException("Invalid token type."); | ||
| } | ||
|
|
||
|
|
||
| public IEnumerable<ValidationResult> Validate(ValidationContext validationContext) | ||
| { | ||
| return KdfSettingsValidator.Validate(Kdf, KdfIterations, KdfMemory, KdfParallelism); | ||
| // 1. Authentication data containing hash and hash at root level check | ||
| if (MasterPasswordAuthentication != null && MasterPasswordHash != null) | ||
| { | ||
| if (MasterPasswordAuthentication.MasterPasswordAuthenticationHash != MasterPasswordHash) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash)} and root level {nameof(MasterPasswordHash)} provided and are not equal. Only provide one.", | ||
| [nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash), nameof(MasterPasswordHash)]); | ||
| } | ||
| } // 1.5 if there is no master password hash that is unacceptable even though they are both optional in the model | ||
| else if (MasterPasswordAuthentication == null && MasterPasswordHash == null) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash)} and {nameof(MasterPasswordHash)} not found on request, one needs to be defined.", | ||
| [nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash), nameof(MasterPasswordHash)]); | ||
| } | ||
|
|
||
| // 2. Validate kdf settings. | ||
| if (MasterPasswordUnlock != null) | ||
| { | ||
| foreach (var validationResult in KdfSettingsValidator.Validate(MasterPasswordUnlock.ToData().Kdf)) | ||
| { | ||
| yield return validationResult; | ||
| } | ||
| } | ||
|
|
||
| if (MasterPasswordAuthentication != null) | ||
| { | ||
| foreach (var validationResult in KdfSettingsValidator.Validate(MasterPasswordAuthentication.ToData().Kdf)) | ||
| { | ||
| yield return validationResult; | ||
| } | ||
| } | ||
|
|
||
| // 3. Validate root kdf values if kdf values are not in the unlock and authentication. | ||
| if (MasterPasswordUnlock == null && MasterPasswordAuthentication == null) | ||
| { | ||
| var hasMissingRequiredKdfInputs = false; | ||
| if (Kdf == null) | ||
| { | ||
| yield return new ValidationResult($"{nameof(Kdf)} not found on RequestModel", [nameof(Kdf)]); | ||
| hasMissingRequiredKdfInputs = true; | ||
| } | ||
| if (KdfIterations == null) | ||
| { | ||
| yield return new ValidationResult($"{nameof(KdfIterations)} not found on RequestModel", [nameof(KdfIterations)]); | ||
| hasMissingRequiredKdfInputs = true; | ||
| } | ||
|
|
||
| if (!hasMissingRequiredKdfInputs) | ||
| { | ||
| foreach (var validationResult in KdfSettingsValidator.Validate( | ||
| Kdf!.Value, | ||
| KdfIterations!.Value, | ||
| KdfMemory, | ||
| KdfParallelism)) | ||
| { | ||
| yield return validationResult; | ||
| } | ||
| } | ||
| } | ||
| else if (MasterPasswordUnlock == null && MasterPasswordAuthentication != null) | ||
| { | ||
| // Authentication provided but Unlock missing | ||
| yield return new ValidationResult($"{nameof(MasterPasswordUnlock)} not found on RequestModel", [nameof(MasterPasswordUnlock)]); | ||
| } | ||
| else if (MasterPasswordUnlock != null && MasterPasswordAuthentication == null) | ||
| { | ||
| // Unlock provided but Authentication missing | ||
| yield return new ValidationResult($"{nameof(MasterPasswordAuthentication)} not found on RequestModel", [nameof(MasterPasswordAuthentication)]); | ||
| } | ||
Patrick-Pimentel-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // 3. Lastly, validate access token type and presence. Must be done last because of yield break. | ||
| RegisterFinishTokenType tokenType; | ||
| var tokenTypeResolved = true; | ||
| try | ||
| { | ||
| tokenType = GetTokenType(); | ||
| } | ||
| catch (InvalidOperationException) | ||
| { | ||
| tokenTypeResolved = false; | ||
| tokenType = default; | ||
| } | ||
|
|
||
| if (!tokenTypeResolved) | ||
| { | ||
| yield return new ValidationResult("No valid registration token provided"); | ||
| yield break; | ||
| } | ||
|
|
||
| switch (tokenType) | ||
| { | ||
| case RegisterFinishTokenType.EmailVerification: | ||
| if (string.IsNullOrEmpty(EmailVerificationToken)) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(EmailVerificationToken)} absent when processing register/finish.", | ||
| [nameof(EmailVerificationToken)]); | ||
| } | ||
| break; | ||
| case RegisterFinishTokenType.OrganizationInvite: | ||
| if (string.IsNullOrEmpty(OrgInviteToken)) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(OrgInviteToken)} absent when processing register/finish.", | ||
| [nameof(OrgInviteToken)]); | ||
| } | ||
| break; | ||
| case RegisterFinishTokenType.OrgSponsoredFreeFamilyPlan: | ||
| if (string.IsNullOrEmpty(OrgSponsoredFreeFamilyPlanToken)) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(OrgSponsoredFreeFamilyPlanToken)} absent when processing register/finish.", | ||
| [nameof(OrgSponsoredFreeFamilyPlanToken)]); | ||
| } | ||
| break; | ||
| case RegisterFinishTokenType.EmergencyAccessInvite: | ||
| if (string.IsNullOrEmpty(AcceptEmergencyAccessInviteToken)) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(AcceptEmergencyAccessInviteToken)} absent when processing register/finish.", | ||
| [nameof(AcceptEmergencyAccessInviteToken)]); | ||
| } | ||
| if (!AcceptEmergencyAccessId.HasValue || AcceptEmergencyAccessId.Value == Guid.Empty) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(AcceptEmergencyAccessId)} absent when processing register/finish.", | ||
| [nameof(AcceptEmergencyAccessId)]); | ||
| } | ||
| break; | ||
| case RegisterFinishTokenType.ProviderInvite: | ||
| if (string.IsNullOrEmpty(ProviderInviteToken)) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(ProviderInviteToken)} absent when processing register/finish.", | ||
| [nameof(ProviderInviteToken)]); | ||
| } | ||
| if (!ProviderUserId.HasValue || ProviderUserId.Value == Guid.Empty) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(ProviderUserId)} absent when processing register/finish.", | ||
| [nameof(ProviderUserId)]); | ||
| } | ||
| break; | ||
| default: | ||
| yield return new ValidationResult("Invalid registration finish request"); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,12 @@ | ||
| ๏ปฟusing System.ComponentModel.DataAnnotations; | ||
| using Bit.Core.KeyManagement.Models.Data; | ||
|
|
||
| namespace Bit.Api.KeyManagement.Models.Requests; | ||
| namespace Bit.Core.KeyManagement.Models.Api.Request; | ||
|
|
||
| /// <summary> | ||
| /// Use this datatype when interfacing with requests to create a separation of concern. | ||
|
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. I don't know if this comment adds anything. The request and data models is rather known separation of concern, but not documented anywhere in contributing docs. Maybe that's the right place to put it ? Opinions @bitwarden/team-key-management-dev ?
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. Agreed, this comment doesn't really add anything.
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. Also, worth to note, there is a PR that mentions this in contributing docs, so this comment and all other about the separation of concern is now redundant. https://github.com/bitwarden/contributing-docs/pull/740/changes#diff-360b9394971135ffb743be955a3db753636cafe84b67bcfdd052b77903511556R164 |
||
| /// See <see cref="MasterPasswordAuthenticationData"/> to use for commands, queries, services. | ||
| /// </summary> | ||
| public class MasterPasswordAuthenticationDataRequestModel | ||
| { | ||
| public required KdfRequestModel Kdf { get; init; } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.