-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Auth/PM-41503 and PM-41533 - Registration - Add open org invite flow support #8159
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
612c702
9273567
d186b50
addcda5
e81c36f
0472aed
bfa5d4f
e23fb66
b4165a4
4a56ca2
6742703
80727c3
684df1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| ο»Ώusing Bit.Core.AdminConsole.Utilities.v2.Results; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.InviteLinks.Interfaces; | ||
|
|
||
| public interface IValidateOrganizationInviteLinkQuery | ||
| { | ||
| /// <summary> | ||
| /// Validates that an open organization invite link is valid and that the email | ||
| /// matches its allowed domains. It does NOT check that the email has been | ||
| /// verified - the caller must check this separately if required. | ||
| /// </summary> | ||
| /// <param name="organizationId">The organization's ID (from the URL path).</param> | ||
| /// <param name="code">The public invite link code.</param> | ||
| /// <param name="email">The registering user's email; checked against the link's AllowedDomains.</param> | ||
| /// <returns> | ||
| /// A successful CommandResult if the link is valid and matches the email; a failed | ||
| /// CommandResult if validation fails or the invite link is otherwise not available. | ||
| /// </returns> | ||
| Task<CommandResult> ValidateAsync(Guid organizationId, Guid code, string email); | ||
|
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. β»οΈ DEBT: This is signature-identical to the pre-existing Details and suggestionBoth live in Task<CommandResult> ValidateAsync(Guid organizationId, Guid code, string email); // new
Task<CommandResult<bool>> ValidateAsync(Guid organizationId, Guid code, string email); // existingThe failure modes read the same at the call site but mean opposite things. For the new query, a disallowed domain is Separately, the link-lookup + Two low-cost options: give this one a name that conveys the fuller scope (e.g. The XML doc already contrasts this query with
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. @eliykat , I could use some feedback here. I think the
Member
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 wasn't familiar with
To address this, please:
// OrganizationInviteLinksController.ValidateEmailDomain
var result = await validateOrganizationInviteLinkQuery.ValidateAsync(model.OrganizationId, model.Code, model.Email);
// Preserve the existing client contract: report the domain check as an IsAllowed boolean
// rather than surfacing a disallowed domain as an error status.
if (result is { IsError: true, AsError: EmailDomainNotAllowed })
{
return TypedResults.Ok(new OrganizationInviteLinkValidateEmailDomainResponseModel(false));
}
return Handle(result, _ =>
TypedResults.Ok(new OrganizationInviteLinkValidateEmailDomainResponseModel(true)));
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 the feedback. Resolved with 4a56ca2 |
||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| ο»Ώusing Bit.Core.AdminConsole.OrganizationFeatures.InviteLinks.Interfaces; | ||
| using Bit.Core.AdminConsole.Repositories; | ||
| using Bit.Core.AdminConsole.Utilities; | ||
| using Bit.Core.AdminConsole.Utilities.v2.Results; | ||
| using Bit.Core.Repositories; | ||
| using OneOf.Types; | ||
|
|
||
| namespace Bit.Core.AdminConsole.OrganizationFeatures.InviteLinks; | ||
|
|
||
| public class ValidateOrganizationInviteLinkQuery( | ||
| IOrganizationInviteLinkRepository organizationInviteLinkRepository, | ||
| IOrganizationRepository organizationRepository) | ||
| : IValidateOrganizationInviteLinkQuery | ||
| { | ||
| public async Task<CommandResult> ValidateAsync(Guid organizationId, Guid code, string email) | ||
| { | ||
| var inviteLink = await organizationInviteLinkRepository.GetByOrganizationIdAsync(organizationId); | ||
| if (inviteLink is null || !inviteLink.CodeMatches(code.ToString())) | ||
| { | ||
| return new InviteLinkNotFound(); | ||
| } | ||
|
|
||
| var organization = await organizationRepository.GetByIdAsync(inviteLink.OrganizationId); | ||
|
JaredSnider-Bitwarden marked this conversation as resolved.
|
||
| if (organization is null or { Enabled: false }) | ||
| { | ||
| return new InviteLinkNotFound(); | ||
| } | ||
|
|
||
| if (!organization.UseInviteLinks) | ||
| { | ||
| return new InviteLinkNotAvailable(); | ||
| } | ||
|
|
||
| if (!InviteLinkDomainValidator.IsEmailDomainAllowed(email, inviteLink.GetAllowedDomains())) | ||
| { | ||
| return new EmailDomainNotAllowed(organization.DisplayName()); | ||
| } | ||
|
|
||
| return new None(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| ο»Ώnamespace Bit.Core.Auth.Models.Api.Request.Accounts; | ||
|
|
||
| /// <summary> | ||
| /// Identifying key for an open organization invite link: the target organization and the | ||
| /// link's bearer code. Sent as the register-finish payload directly, and extended by | ||
| /// <see cref="RegisterStartOpenOrgInviteRequestModel"/> with a sealed data blob at register-start. | ||
| /// </summary> | ||
| public class OpenOrgInviteRequestModel | ||
| { | ||
| public required Guid OrganizationId { get; set; } | ||
|
|
||
| public required Guid Code { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,8 @@ public class RegisterFinishRequestModel : IValidatableObject | |
|
|
||
| public string? SalesAssistedToken { get; set; } | ||
|
|
||
| public OpenOrgInviteRequestModel? OpenOrgInvite { get; set; } | ||
|
|
||
| public User ToUser(bool isV2Encryption) | ||
| { | ||
| // TODO remove IsV2Encryption bool and simplify logic below after a compatibility period - once V2 accounts are supported | ||
|
|
@@ -245,7 +247,8 @@ public IEnumerable<ValidationResult> Validate(ValidationContext validationContex | |
| [nameof(AccountKeys.PublicKeyEncryptionKeyPair.PublicKey), nameof(AccountKeys.PublicKeyEncryptionKeyPair.WrappedPrivateKey)]); | ||
| } | ||
|
|
||
| // 4. Lastly, validate access token type and presence. Must be done last because of yield break. | ||
| // 4. Resolve access token type; short-circuit if no valid token was provided. | ||
| // Must be done after all other checks above because of the yield break below. | ||
|
Comment on lines
+250
to
+251
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. Less is more. |
||
| RegisterFinishTokenType tokenType; | ||
| var tokenTypeResolved = true; | ||
| try | ||
|
|
@@ -264,6 +267,15 @@ public IEnumerable<ValidationResult> Validate(ValidationContext validationContex | |
| yield break; | ||
| } | ||
|
|
||
| // 5. OpenOrgInvite is only compatible with the EmailVerification token type. | ||
| if (OpenOrgInvite is not null && tokenType != RegisterFinishTokenType.EmailVerification) | ||
| { | ||
| yield return new ValidationResult( | ||
| $"{nameof(OpenOrgInvite)} is only valid with an {nameof(EmailVerificationToken)}.", | ||
| [nameof(OpenOrgInvite)]); | ||
| } | ||
|
|
||
| // 6. Validate token presence per resolved type. | ||
| switch (tokenType) | ||
| { | ||
| case RegisterFinishTokenType.EmailVerification: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| ο»Ώusing System.ComponentModel.DataAnnotations; | ||
|
|
||
| namespace Bit.Core.Auth.Models.Api.Request.Accounts; | ||
|
|
||
| /// <summary> | ||
| /// Register-start payload for an open organization invite link: the {organizationId, code} | ||
| /// reference (inherited from <see cref="OpenOrgInviteRequestModel"/>) plus the opaque SDK-produced | ||
| /// sealed blob that is echoed to the verification email URL to enable the registration finish tab | ||
| /// to securely reconstitute the open organization invite data. | ||
| /// </summary> | ||
| public class RegisterStartOpenOrgInviteRequestModel : OpenOrgInviteRequestModel | ||
| { | ||
| private const int SealedOpenOrgInviteDataMaxLength = 4096; | ||
|
|
||
| [Required] | ||
| [MaxLength(SealedOpenOrgInviteDataMaxLength)] | ||
| public required string SealedOpenOrgInviteData { get; set; } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude xmldoc :((( Way too wordy and too much context associated with this specific PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback. Resolved with 4a56ca2