From 09aea4ed38d1d8c911cd4188cb6541a0d3df4856 Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Fri, 4 Dec 2020 16:45:54 -0600 Subject: [PATCH] [Bug] Improve SSO user provision flow (#1022) * Initial commit of provisioning updates * Updated strings * removed extra BANG * Separated orgUsers db lookup - prioritized existing user Id * Updated create sso record method // Added sproc for org/email retrieval --- .../src/Sso/Controllers/AccountController.cs | 179 +++++++++--------- .../IOrganizationUserRepository.cs | 1 + .../SqlServer/OrganizationUserRepository.cs | 13 ++ src/Core/Resources/SharedResources.en.resx | 6 +- src/Sql/Sql.sqlproj | 2 + ...nizationUser_ReadByOrganizationIdEmail.sql | 17 ++ .../2020_12_04_00_OrgUserReadByOrgEmail.sql | 24 +++ 7 files changed, 149 insertions(+), 93 deletions(-) create mode 100644 src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdEmail.sql create mode 100644 util/Migrator/DbScripts/2020_12_04_00_OrgUserReadByOrgEmail.sql diff --git a/bitwarden_license/src/Sso/Controllers/AccountController.cs b/bitwarden_license/src/Sso/Controllers/AccountController.cs index 451b1260f0..ac11640f58 100644 --- a/bitwarden_license/src/Sso/Controllers/AccountController.cs +++ b/bitwarden_license/src/Sso/Controllers/AccountController.cs @@ -357,13 +357,8 @@ namespace Bit.Sso.Controllers { email = providerUserId; } - - Guid? orgId = null; - if (Guid.TryParse(provider, out var oId)) - { - orgId = oId; - } - else + + if (!Guid.TryParse(provider, out var orgId)) { // TODO: support non-org (server-wide) SSO in the future? throw new Exception(_i18nService.T("SSOProviderIsNotAnOrgId", provider)); @@ -407,106 +402,99 @@ namespace Bit.Sso.Controllers } OrganizationUser orgUser = null; - if (orgId.HasValue) + var organization = await _organizationRepository.GetByIdAsync(orgId); + if (organization == null) { - var organization = await _organizationRepository.GetByIdAsync(orgId.Value); - if (organization == null) - { - throw new Exception(_i18nService.T("CouldNotFindOrganization", orgId)); - } - - if (existingUser != null) - { - var orgUsers = await _organizationUserRepository.GetManyByUserAsync(existingUser.Id); - orgUser = orgUsers.SingleOrDefault(u => u.OrganizationId == orgId.Value && - u.Status != OrganizationUserStatusType.Invited); - } + throw new Exception(_i18nService.T("CouldNotFindOrganization", orgId)); + } + + // Try to find OrgUser via existing User Id (accepted/confirmed user) + if (existingUser != null) + { + var orgUsersByUserId = await _organizationUserRepository.GetManyByUserAsync(existingUser.Id); + orgUser = orgUsersByUserId.SingleOrDefault(u => u.OrganizationId == orgId); + } + // If no Org User found by Existing User Id - search all organization users via email + orgUser ??= await _organizationUserRepository.GetByOrganizationEmailAsync(orgId, email); + + // All Existing User flows handled below + if (existingUser != null) + { if (orgUser == null) { - if (organization.Seats.HasValue) - { - var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(orgId.Value); - var availableSeats = organization.Seats.Value - userCount; - if (availableSeats < 1) - { - // No seats are available - throw new Exception(_i18nService.T("NoSeatsAvailable", organization.Name)); - } - } + // Org User is not created - no invite has been sent + throw new Exception(_i18nService.T("UserAlreadyExistsInviteProcess")); + } + + if (orgUser.Status == OrganizationUserStatusType.Invited) + { + // Org User is invited - they must manually accept the invite via email and authenticate with MP + throw new Exception(_i18nService.T("UserAlreadyInvited", email, organization.Name)); + } + + // Accepted or Confirmed - create SSO link and return; + await CreateSsoUserRecord(providerUserId, existingUser.Id, orgId); + return existingUser; + } - // Make sure user is not already invited to this org - var existingOrgUserCount = await _organizationUserRepository.GetCountByOrganizationAsync( - orgId.Value, email, false); - if (existingOrgUserCount > 0) - { - throw new Exception(_i18nService.T("UserAlreadyInvited", email, organization.Name)); - } + // Before any user creation - if Org User doesn't exist at this point - make sure there are enough seats to add one + if (orgUser == null && organization.Seats.HasValue) + { + var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(orgId); + var availableSeats = organization.Seats.Value - userCount; + if (availableSeats < 1) + { + throw new Exception(_i18nService.T("NoSeatsAvailable", organization.Name)); } } - User user = null; + // Create user record - all existing user flows are handled above + var user = new User + { + Name = name, + Email = email, + ApiKey = CoreHelpers.SecureRandomString(30) + }; + await _userService.RegisterUserAsync(user); + + // If the organization has 2fa policy enabled, make sure to default jit user 2fa to email + var twoFactorPolicy = + await _policyRepository.GetByOrganizationIdTypeAsync(orgId, PolicyType.TwoFactorAuthentication); + if (twoFactorPolicy != null && twoFactorPolicy.Enabled) + { + user.SetTwoFactorProviders(new Dictionary + { + [TwoFactorProviderType.Email] = new TwoFactorProvider + { + MetaData = new Dictionary { ["Email"] = user.Email.ToLowerInvariant() }, + Enabled = true + } + }); + await _userService.UpdateTwoFactorProviderAsync(user, TwoFactorProviderType.Email); + } + + // Create Org User if null or else update existing Org User if (orgUser == null) { - if (existingUser != null) + orgUser = new OrganizationUser { - // TODO: send an email inviting this user to link SSO to their account? - throw new Exception(_i18nService.T("UserAlreadyExistsUseLinkViaSso")); - } - - // Create user record - user = new User - { - Name = name, - Email = email, - ApiKey = CoreHelpers.SecureRandomString(30) + OrganizationId = orgId, + UserId = user.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Invited }; - await _userService.RegisterUserAsync(user); - - if (orgId.HasValue) - { - // If the organization has 2fa policy enabled, make sure to default jit user 2fa to email - var twoFactorPolicy = - await _policyRepository.GetByOrganizationIdTypeAsync(orgId.Value, PolicyType.TwoFactorAuthentication); - if (twoFactorPolicy != null && twoFactorPolicy.Enabled) - { - user.SetTwoFactorProviders(new Dictionary - { - - [TwoFactorProviderType.Email] = new TwoFactorProvider - { - MetaData = new Dictionary { ["Email"] = user.Email.ToLowerInvariant() }, - Enabled = true - } - }); - await _userService.UpdateTwoFactorProviderAsync(user, TwoFactorProviderType.Email); - } - // Create organization user record - orgUser = new OrganizationUser - { - OrganizationId = orgId.Value, - UserId = user.Id, - Type = OrganizationUserType.User, - Status = OrganizationUserStatusType.Invited - }; - await _organizationUserRepository.CreateAsync(orgUser); - } + await _organizationUserRepository.CreateAsync(orgUser); } else { - // Since the user is already a member of this organization, let's link their existing user account - user = existingUser; + orgUser.UserId = user.Id; + await _organizationUserRepository.ReplaceAsync(orgUser); } - + // Create sso user record - var ssoUser = new SsoUser - { - ExternalId = providerUserId, - UserId = user.Id, - OrganizationId = orgId - }; - await _ssoUserRepository.CreateAsync(ssoUser); - + await CreateSsoUserRecord(providerUserId, user.Id, orgId); + return user; } @@ -554,6 +542,17 @@ namespace Bit.Sso.Controllers return null; } + private async Task CreateSsoUserRecord(string providerUserId, Guid userId, Guid orgId) + { + var ssoUser = new SsoUser + { + ExternalId = providerUserId, + UserId = userId, + OrganizationId = orgId + }; + await _ssoUserRepository.CreateAsync(ssoUser); + } + private void ProcessLoginCallback(AuthenticateResult externalResult, List localClaims, AuthenticationProperties localSignInProps) { diff --git a/src/Core/Repositories/IOrganizationUserRepository.cs b/src/Core/Repositories/IOrganizationUserRepository.cs index 6bd3abf783..92dcf595dc 100644 --- a/src/Core/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/Repositories/IOrganizationUserRepository.cs @@ -27,5 +27,6 @@ namespace Bit.Core.Repositories Task CreateAsync(OrganizationUser obj, IEnumerable collections); Task ReplaceAsync(OrganizationUser obj, IEnumerable collections); Task> GetManyByManyUsersAsync(IEnumerable userIds); + Task GetByOrganizationEmailAsync(Guid organizationId, string email); } } diff --git a/src/Core/Repositories/SqlServer/OrganizationUserRepository.cs b/src/Core/Repositories/SqlServer/OrganizationUserRepository.cs index 45db606312..76407cc7d5 100644 --- a/src/Core/Repositories/SqlServer/OrganizationUserRepository.cs +++ b/src/Core/Repositories/SqlServer/OrganizationUserRepository.cs @@ -244,5 +244,18 @@ namespace Bit.Core.Repositories.SqlServer return results.ToList(); } } + + public async Task GetByOrganizationEmailAsync(Guid organizationId, string email) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + "[dbo].[OrganizationUser_ReadByOrganizationIdEmail]", + new { OrganizationId = organizationId, Email = email }, + commandType: CommandType.StoredProcedure); + + return results.SingleOrDefault(); + } + } } } diff --git a/src/Core/Resources/SharedResources.en.resx b/src/Core/Resources/SharedResources.en.resx index 72c400e741..56ff3d83bb 100644 --- a/src/Core/Resources/SharedResources.en.resx +++ b/src/Core/Resources/SharedResources.en.resx @@ -521,10 +521,10 @@ No seats available for organization, '{0}' - User, '{0}', has already been invited to this organization, '{1}' + User, '{0}', has already been invited to this organization, '{1}'. Accept the invite in order to log in with SSO. - - User already exists, please link account to SSO after logging in + + In order to join this organization, contact an admin to send you an invite and follow the instructions within to accept. Redirect GET diff --git a/src/Sql/Sql.sqlproj b/src/Sql/Sql.sqlproj index 1b38aa31b9..9d500ae401 100644 --- a/src/Sql/Sql.sqlproj +++ b/src/Sql/Sql.sqlproj @@ -294,5 +294,7 @@ + + diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdEmail.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdEmail.sql new file mode 100644 index 0000000000..313deff1bc --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_ReadByOrganizationIdEmail.sql @@ -0,0 +1,17 @@ +CREATE PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdEmail] + @OrganizationId UNIQUEIDENTIFIER, + @Email NVARCHAR(50) +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[OrganizationUserView] + WHERE + [OrganizationId] = @OrganizationId + AND [Email] IS NOT NULL + AND @Email IS NOT NULL + AND [Email] = @Email +END \ No newline at end of file diff --git a/util/Migrator/DbScripts/2020_12_04_00_OrgUserReadByOrgEmail.sql b/util/Migrator/DbScripts/2020_12_04_00_OrgUserReadByOrgEmail.sql new file mode 100644 index 0000000000..e7bcd861d6 --- /dev/null +++ b/util/Migrator/DbScripts/2020_12_04_00_OrgUserReadByOrgEmail.sql @@ -0,0 +1,24 @@ +IF OBJECT_ID('[dbo].[OrganizationUser_ReadByOrganizationEmail]') IS NOT NULL +BEGIN + DROP PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationEmail] +END +GO + +CREATE PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdEmail] + @OrganizationId UNIQUEIDENTIFIER, + @Email NVARCHAR(50) +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[OrganizationUserView] + WHERE + [OrganizationId] = @OrganizationId + AND [Email] IS NOT NULL + AND @Email IS NOT NULL + AND [Email] = @Email +END +GO