mirror of
https://github.com/bitwarden/server.git
synced 2025-04-05 05:00:19 -05:00
Provider qa feedback (#1501)
* Title case buttons * Throw if provider tries to add a non-business organization * Allow only one admin OR owner roll in a free org per user Boolean operators were not properly assocated and ownership of an org was precluding confirmation into any other organization * Limit email length * Require email domain with top level domain * Do not allow email domains to end in invalid characters * Fix free org tests
This commit is contained in:
Matt Gibson
GitHub
parent
b726b08ea1
commit
5dc6013e37
@ -21,6 +21,8 @@ namespace Bit.CommCore.Services
|
||||
{
|
||||
public class ProviderService : IProviderService
|
||||
{
|
||||
public static PlanType[] ProviderDisllowedOrganizationTypes = new[] { PlanType.Free, PlanType.FamiliesAnnually, PlanType.FamiliesAnnually2019 };
|
||||
|
||||
private readonly IDataProtector _dataProtector;
|
||||
private readonly IMailService _mailService;
|
||||
private readonly IEventService _eventService;
|
||||
@ -380,6 +382,9 @@ namespace Bit.CommCore.Services
|
||||
throw new BadRequestException("Organization already belongs to a provider.");
|
||||
}
|
||||
|
||||
var organization = await _organizationRepository.GetByIdAsync(organizationId);
|
||||
ThrowOnInvalidPlanType(organization.PlanType);
|
||||
|
||||
var providerOrganization = new ProviderOrganization
|
||||
{
|
||||
ProviderId = providerId,
|
||||
@ -394,6 +399,8 @@ namespace Bit.CommCore.Services
|
||||
public async Task<ProviderOrganization> CreateOrganizationAsync(Guid providerId,
|
||||
OrganizationSignup organizationSignup, string clientOwnerEmail, User user)
|
||||
{
|
||||
ThrowOnInvalidPlanType(organizationSignup.Plan);
|
||||
|
||||
var (organization, _) = await _organizationService.SignUpAsync(organizationSignup, true);
|
||||
|
||||
var providerOrganization = new ProviderOrganization
|
||||
@ -487,5 +494,13 @@ namespace Bit.CommCore.Services
|
||||
var confirmedOwnersIds = confirmedOwners.Select(u => u.Id);
|
||||
return confirmedOwnersIds.Except(providerUserIds).Any();
|
||||
}
|
||||
|
||||
private void ThrowOnInvalidPlanType(PlanType requestedType)
|
||||
{
|
||||
if (ProviderDisllowedOrganizationTypes.Contains(requestedType))
|
||||
{
|
||||
throw new BadRequestException($"Providers cannot manage organizations with the requested plan type ({requestedType}). Only Teams and Enterprise accounts are allowed.");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -8,7 +8,7 @@
|
||||
<tr style="margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;">
|
||||
<td class="content-block" style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; margin: 0; -webkit-font-smoothing: antialiased; padding: 0 0 10px; -webkit-text-size-adjust: none; text-align: center;" valign="top" align="center">
|
||||
<a href="{{{Url}}}" clicktracking=off target="_blank" style="color: #ffffff; text-decoration: none; text-align: center; cursor: pointer; display: inline-block; border-radius: 5px; background-color: #175DDC; border-color: #175DDC; border-style: solid; border-width: 10px 20px; margin: 0; font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; line-height: 25px; -webkit-font-smoothing: antialiased; -webkit-text-size-adjust: none;">
|
||||
Set up Provider Now
|
||||
Set Up Provider Now
|
||||
</a>
|
||||
</td>
|
||||
</tr>
|
||||
|
||||
@ -6,6 +6,7 @@ namespace Bit.Core.Models.Api.Request
|
||||
public class ProviderOrganizationCreateRequestModel
|
||||
{
|
||||
[Required]
|
||||
[StringLength(256)]
|
||||
[StrictEmailAddress]
|
||||
public string ClientOwnerEmail { get; set; }
|
||||
[Required]
|
||||
|
||||
@ -1488,8 +1488,8 @@ namespace Bit.Core.Services
|
||||
var orgUsers = keyedOrganizationUsers.GetValueOrDefault(user.Id, new List<OrganizationUser>());
|
||||
try
|
||||
{
|
||||
if (organization.PlanType == PlanType.Free && orgUser.Type == OrganizationUserType.Admin
|
||||
|| orgUser.Type == OrganizationUserType.Owner)
|
||||
if (organization.PlanType == PlanType.Free && (orgUser.Type == OrganizationUserType.Admin
|
||||
|| orgUser.Type == OrganizationUserType.Owner))
|
||||
{
|
||||
// Since free organizations only supports a few users there is not much point in avoiding N+1 queries for this.
|
||||
var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(user.Id);
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
using System.ComponentModel.DataAnnotations;
|
||||
using System.Text.RegularExpressions;
|
||||
using MimeKit;
|
||||
|
||||
namespace Bit.Core.Utilities
|
||||
@ -25,7 +26,12 @@ namespace Bit.Core.Utilities
|
||||
return false;
|
||||
}
|
||||
}
|
||||
catch (ParseException e)
|
||||
catch (ParseException)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!Regex.IsMatch(emailAddress, @"@.+\.[A-Za-z0-9]+$"))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
@ -584,10 +584,12 @@ namespace Bit.Core.Test.Services
|
||||
() => sutProvider.Sut.ConfirmUserAsync(confirmingUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService));
|
||||
Assert.Contains("User not valid.", exception.Message);
|
||||
}
|
||||
|
||||
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
|
||||
public async Task ConfirmUser_AlreadyAdmin(Organization org, OrganizationUser confirmingUser,
|
||||
[OrganizationUser(OrganizationUserStatusType.Accepted, OrganizationUserType.Admin)]OrganizationUser orgUser, User user,
|
||||
|
||||
[Theory]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, OrganizationUserType.Owner)]
|
||||
public async Task ConfirmUserToFree_AlreadyFreeAdminOrOwner_Throws(OrganizationUserType userType, Organization org, OrganizationUser confirmingUser,
|
||||
[OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user,
|
||||
string key, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
|
||||
@ -598,6 +600,7 @@ namespace Bit.Core.Test.Services
|
||||
org.PlanType = PlanType.Free;
|
||||
orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id;
|
||||
orgUser.UserId = user.Id;
|
||||
orgUser.Type = userType;
|
||||
organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] {orgUser});
|
||||
organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(orgUser.UserId.Value).Returns(1);
|
||||
organizationRepository.GetByIdAsync(org.Id).Returns(org);
|
||||
@ -608,6 +611,55 @@ namespace Bit.Core.Test.Services
|
||||
Assert.Contains("User can only be an admin of one free organization.", exception.Message);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.Custom, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.Custom, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseAnnually, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseAnnually, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseAnnually2019, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseAnnually2019, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseMonthly, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseMonthly, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseMonthly2019, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.EnterpriseMonthly2019, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.FamiliesAnnually, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.FamiliesAnnually, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.FamiliesAnnually2019, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.FamiliesAnnually2019, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsAnnually, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsAnnually, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsAnnually2019, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsAnnually2019, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsMonthly, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsMonthly, OrganizationUserType.Owner)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsMonthly2019, OrganizationUserType.Admin)]
|
||||
[InlineCustomAutoData(new[] { typeof(SutProviderCustomization) }, PlanType.TeamsMonthly2019, OrganizationUserType.Owner)]
|
||||
public async Task ConfirmUserToNonFree_AlreadyFreeAdminOrOwner_DoesNotThrow(PlanType planType, OrganizationUserType orgUserType, Organization org, OrganizationUser confirmingUser,
|
||||
[OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, User user,
|
||||
string key, SutProvider<OrganizationService> sutProvider)
|
||||
{
|
||||
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
|
||||
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
|
||||
var userService = Substitute.For<IUserService>();
|
||||
var userRepository = sutProvider.GetDependency<IUserRepository>();
|
||||
|
||||
org.PlanType = planType;
|
||||
orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id;
|
||||
orgUser.UserId = user.Id;
|
||||
orgUser.Type = orgUserType;
|
||||
organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser });
|
||||
organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(orgUser.UserId.Value).Returns(1);
|
||||
organizationRepository.GetByIdAsync(org.Id).Returns(org);
|
||||
userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user });
|
||||
|
||||
await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService);
|
||||
|
||||
await sutProvider.GetDependency<IEventService>().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed);
|
||||
await sutProvider.GetDependency<IMailService>().Received(1).SendOrganizationConfirmedEmailAsync(org.Name, user.Email);
|
||||
await organizationUserRepository.Received(1).ReplaceManyAsync(Arg.Is<List<OrganizationUser>>(users => users.Contains(orgUser) && users.Count == 1));
|
||||
}
|
||||
|
||||
|
||||
[Theory, CustomAutoData(typeof(SutProviderCustomization))]
|
||||
public async Task ConfirmUser_SingleOrgPolicy(Organization org, OrganizationUser confirmingUser,
|
||||
[OrganizationUser(OrganizationUserStatusType.Accepted)]OrganizationUser orgUser, User user,
|
||||
|
||||
@ -20,25 +20,29 @@ namespace Bit.Core.Test.Utilities
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(null)] // null
|
||||
[InlineData("hello@world.com\t")] // trailing tab char
|
||||
[InlineData("\thello@world.com")] // leading tab char
|
||||
[InlineData("hel\tlo@world.com")] // local-part tab char
|
||||
[InlineData("hello@world.com\b")] // trailing backspace char
|
||||
[InlineData("\" \"hello@world.com")] // leading spaces in quotes
|
||||
[InlineData("hello@world.com\" \"")] // trailing spaces in quotes
|
||||
[InlineData("hel\" \"lo@world.com")] // local-part spaces in quotes
|
||||
[InlineData("hello there@world.com")] // unescaped unquoted spaces
|
||||
[InlineData("Hello <hello@world.com>")] // friendly from
|
||||
[InlineData("<hello@world.com>")] // wrapped angle brackets
|
||||
[InlineData("hello(com)there@world.com")] // comment
|
||||
[InlineData("hello@world.com.")] // trailing period
|
||||
[InlineData(".hello@world.com")] // leading period
|
||||
[InlineData("hello@world.com;")] // trailing semicolon
|
||||
[InlineData(";hello@world.com")] // leading semicolon
|
||||
[InlineData(null)] // null
|
||||
[InlineData("hello@world.com\t")] // trailing tab char
|
||||
[InlineData("\thello@world.com")] // leading tab char
|
||||
[InlineData("hel\tlo@world.com")] // local-part tab char
|
||||
[InlineData("hello@world.com\b")] // trailing backspace char
|
||||
[InlineData("\" \"hello@world.com")] // leading spaces in quotes
|
||||
[InlineData("hello@world.com\" \"")] // trailing spaces in quotes
|
||||
[InlineData("hel\" \"lo@world.com")] // local-part spaces in quotes
|
||||
[InlineData("hello there@world.com")] // unescaped unquoted spaces
|
||||
[InlineData("Hello <hello@world.com>")] // friendly from
|
||||
[InlineData("<hello@world.com>")] // wrapped angle brackets
|
||||
[InlineData("hello(com)there@world.com")] // comment
|
||||
[InlineData("hello@world.com.")] // trailing period
|
||||
[InlineData(".hello@world.com")] // leading period
|
||||
[InlineData("hello@world.com;")] // trailing semicolon
|
||||
[InlineData(";hello@world.com")] // leading semicolon
|
||||
[InlineData("hello@world.com; hello@world.com")] // semicolon separated list
|
||||
[InlineData("hello@world.com, hello@world.com")] // comma separated list
|
||||
|
||||
[InlineData("hellothere@worldcom")] // dotless domain
|
||||
[InlineData("hello.there@worldcom")] // dotless domain
|
||||
[InlineData("hellothere@.worldcom")] // domain beginning with dot
|
||||
[InlineData("hellothere@worldcom.")] // domain ending in dot
|
||||
[InlineData("hellothere@world.com-")] // domain ending in hyphen
|
||||
public void IsValid_ReturnsFalseWhenInvalid(string email)
|
||||
{
|
||||
var sut = new StrictEmailAddressAttribute();
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user