Skip to content
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

[PM-14489] Limit managed users to confirmed or revoked status #4978

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public interface IGetOrganizationUsersManagementStatusQuery
/// <remarks>
/// A managed user is a user whose email domain matches one of the Organization's verified domains.
/// The organization must be enabled and be on an Enterprise plan.
/// The user must be a member of the organization with a confirmed or revoked status.
/// </remarks>
/// <returns>
/// A dictionary containing the OrganizationUserId and a boolean indicating if the user is managed by the organization.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ public interface IOrganizationRepository : IRepository<Organization, Guid>
Task<IEnumerable<string>> GetOwnerEmailAddressesById(Guid organizationId);

/// <summary>
/// Gets the organizations that have a verified domain matching the user's email domain.
/// Retrieves organizations where the user's email domain matches a verified organization domain.
/// </summary>
/// <remarks>
/// Only returns organizations where:
/// 1. The domain in the user's email matches a verified organization domain
/// 2. The user is an existing organization member with either 'Confirmed' or 'Revoked' status
/// </remarks>
Task<ICollection<Organization>> GetByVerifiedUserEmailDomainAsync(Guid userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,8 @@ UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid userId,
/// <summary>
/// Returns a list of OrganizationUsers with email domains that match one of the Organization's claimed domains.
/// </summary>
/// <remarks>
/// Only returns users with a confirmed or revoked status.
/// </remarks>
Task<ICollection<OrganizationUser>> GetManyByOrganizationWithClaimedDomainsAsync(Guid organizationId);
}
6 changes: 4 additions & 2 deletions src/Core/Services/IUserService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ Task<IdentityResult> UpdatePasswordHash(User user, string newPassword,
/// Indicates if the user is managed by any organization.
/// </summary>
/// <remarks>
/// A user is considered managed by an organization if their email domain matches one of the verified domains of that organization, and the user is a member of it.
/// The organization must be enabled and able to have verified domains.
/// A user is considered "managed" when they meet all these criteria:
/// 1. Their email domain matches a verified domain of an organization
/// 2. They are a member of that organization (with confirmed or revoked status)
/// 3. The organization is enabled and has domain verification capabilities
/// </remarks>
/// <returns>
/// False if the Account Deprovisioning feature flag is disabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,9 @@ join ou in dbContext.OrganizationUsers on u.Id equals ou.UserId
join o in dbContext.Organizations on ou.OrganizationId equals o.Id
join od in dbContext.OrganizationDomains on ou.OrganizationId equals od.OrganizationId
where u.Id == userId
&& od.VerifiedDate != null
&& u.Email.ToLower().EndsWith("@" + od.DomainName.ToLower())
&& (ou.Status == OrganizationUserStatusType.Confirmed || ou.Status == OrganizationUserStatusType.Revoked)
&& od.VerifiedDate != null
&& u.Email.ToLower().EndsWith("@" + od.DomainName.ToLower())
select o;

return await query.ToArrayAsync();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Bit.Core.Entities;
using Bit.Core.Enums;

namespace Bit.Infrastructure.EntityFramework.Repositories.Queries;

Expand All @@ -16,8 +17,9 @@
var query = from ou in dbContext.OrganizationUsers
join u in dbContext.Users on ou.UserId equals u.Id
where ou.OrganizationId == _organizationId
&& dbContext.OrganizationDomains
.Any(od => od.OrganizationId == _organizationId &&
&& (ou.Status == OrganizationUserStatusType.Confirmed || ou.Status == OrganizationUserStatusType.Revoked)
&& dbContext.OrganizationDomains
.Any(od => od.OrganizationId == _organizationId &&

Check warning on line 22 in src/Infrastructure.EntityFramework/AdminConsole/Repositories/Queries/OrganizationUserReadByClaimedOrganizationDomainsQuery.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.EntityFramework/AdminConsole/Repositories/Queries/OrganizationUserReadByClaimedOrganizationDomainsQuery.cs#L20-L22

Added lines #L20 - L22 were not covered by tests
od.VerifiedDate != null &&
u.Email.ToLower().EndsWith("@" + od.DomainName.ToLower()))
select ou;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ BEGIN
FROM [dbo].[OrganizationUserView] OU
INNER JOIN [dbo].[UserView] U ON OU.[UserId] = U.[Id]
WHERE OU.[OrganizationId] = @OrganizationId
AND (
OU.[Status] = 2 -- Confirmed
OR OU.[Status] = -1 -- Revoked
)
AND EXISTS (
SELECT 1
FROM [dbo].[OrganizationDomainView] OD
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ BEGIN
INNER JOIN [dbo].[OrganizationView] O ON OU.[OrganizationId] = O.[Id]
INNER JOIN [dbo].[OrganizationDomainView] OD ON OU.[OrganizationId] = OD.[OrganizationId]
WHERE U.[Id] = @UserId
AND (
OU.[Status] = 2 -- Confirmed
OR OU.[Status] = -1 -- Revoked
)
AND OD.[VerifiedDate] IS NOT NULL
AND U.[Email] LIKE '%@' + OD.[DomainName];
END
Original file line number Diff line number Diff line change
Expand Up @@ -253,4 +253,80 @@ public async Task GetByVerifiedUserEmailDomainAsync_WithNonExistentUser_ReturnsE

Assert.Empty(result);
}

[DatabaseTheory, DatabaseData]
public async Task GetByClaimedUserDomainAsync_WithNonConfirmedOrRevokedUsers_ReturnsEmpty(
IUserRepository userRepository,
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
IOrganizationDomainRepository organizationDomainRepository)
{
var id = Guid.NewGuid();
var domainName = $"{id}.example.com";

var user1 = await userRepository.CreateAsync(new User
{
Name = "Test User 1",
Email = $"test+{id}@{domainName}",
ApiKey = "TEST",
SecurityStamp = "stamp",
Kdf = KdfType.PBKDF2_SHA256,
KdfIterations = 1,
KdfMemory = 2,
KdfParallelism = 3
});

var user2 = await userRepository.CreateAsync(new User
{
Name = "Test User 2",
Email = $"test+{id}@x-{domainName}", // Different domain
ApiKey = "TEST",
SecurityStamp = "stamp",
Kdf = KdfType.PBKDF2_SHA256,
KdfIterations = 1,
KdfMemory = 2,
KdfParallelism = 3
});

var organization = await organizationRepository.CreateAsync(new Organization
{
Name = $"Test Org {id}",
BillingEmail = user1.Email, // TODO: EF does not enforce this being NOT NULl
Plan = "Test", // TODO: EF does not enforce this being NOT NULl
PrivateKey = "privatekey",
});

var organizationDomain = new OrganizationDomain
{
OrganizationId = organization.Id,
DomainName = domainName,
Txt = "btw+12345",
};
organizationDomain.SetVerifiedDate();
organizationDomain.SetNextRunDate(12);
organizationDomain.SetJobRunCount();
await organizationDomainRepository.CreateAsync(organizationDomain);

await organizationUserRepository.CreateAsync(new OrganizationUser
{
OrganizationId = organization.Id,
UserId = user1.Id,
Status = OrganizationUserStatusType.Invited,
ResetPasswordKey = "resetpasswordkey1",
});

await organizationUserRepository.CreateAsync(new OrganizationUser
{
OrganizationId = organization.Id,
UserId = user2.Id,
Status = OrganizationUserStatusType.Accepted,
ResetPasswordKey = "resetpasswordkey1",
});

var user1Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user1.Id);
var user2Response = await organizationRepository.GetByVerifiedUserEmailDomainAsync(user2.Id);

Assert.Empty(user1Response);
Assert.Empty(user2Response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,79 @@ await organizationUserRepository.CreateAsync(new OrganizationUser
Assert.Single(responseModel);
Assert.Equal(orgUser1.Id, responseModel.Single().Id);
}

[DatabaseTheory, DatabaseData]
public async Task GetManyByOrganizationWithClaimedDomainsAsync_WithNonConfirmedOrRevokedUsers_ReturnsEmpty(
IUserRepository userRepository,
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
IOrganizationDomainRepository organizationDomainRepository)
{
var id = Guid.NewGuid();
var domainName = $"{id}.example.com";

var user1 = await userRepository.CreateAsync(new User
{
Name = "Test User 1",
Email = $"test+{id}@{domainName}",
ApiKey = "TEST",
SecurityStamp = "stamp",
Kdf = KdfType.PBKDF2_SHA256,
KdfIterations = 1,
KdfMemory = 2,
KdfParallelism = 3
});

var user2 = await userRepository.CreateAsync(new User
{
Name = "Test User 2",
Email = $"test+{id}@x-{domainName}", // Different domain
ApiKey = "TEST",
SecurityStamp = "stamp",
Kdf = KdfType.PBKDF2_SHA256,
KdfIterations = 1,
KdfMemory = 2,
KdfParallelism = 3
});

var organization = await organizationRepository.CreateAsync(new Organization
{
Name = $"Test Org {id}",
BillingEmail = user1.Email, // TODO: EF does not enforce this being NOT NULl
Plan = "Test", // TODO: EF does not enforce this being NOT NULl
PrivateKey = "privatekey",
});

var organizationDomain = new OrganizationDomain
{
OrganizationId = organization.Id,
DomainName = domainName,
Txt = "btw+12345",
};
organizationDomain.SetVerifiedDate();
organizationDomain.SetNextRunDate(12);
organizationDomain.SetJobRunCount();
await organizationDomainRepository.CreateAsync(organizationDomain);

var orgUser1 = await organizationUserRepository.CreateAsync(new OrganizationUser
{
OrganizationId = organization.Id,
UserId = user1.Id,
Status = OrganizationUserStatusType.Invited,
ResetPasswordKey = "resetpasswordkey1",
});

await organizationUserRepository.CreateAsync(new OrganizationUser
{
OrganizationId = organization.Id,
UserId = user2.Id,
Status = OrganizationUserStatusType.Accepted,
ResetPasswordKey = "resetpasswordkey1",
});

var responseModel = await organizationUserRepository.GetManyByOrganizationWithClaimedDomainsAsync(organization.Id);

Assert.NotNull(responseModel);
Assert.Empty(responseModel);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_ReadByOrganizationIdWithClaimedDomains]
@OrganizationId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON;

SELECT OU.*
FROM [dbo].[OrganizationUserView] OU
INNER JOIN [dbo].[UserView] U ON OU.[UserId] = U.[Id]
WHERE OU.[OrganizationId] = @OrganizationId
AND (
OU.[Status] = 2 -- Confirmed
OR OU.[Status] = -1 -- Revoked
)
AND EXISTS (
SELECT 1
FROM [dbo].[OrganizationDomainView] OD
WHERE OD.[OrganizationId] = @OrganizationId
AND OD.[VerifiedDate] IS NOT NULL
AND U.[Email] LIKE '%@' + OD.[DomainName]
);
END
GO

CREATE OR ALTER PROCEDURE [dbo].[Organization_ReadByClaimedUserEmailDomain]
@UserId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON;

SELECT O.*
FROM [dbo].[UserView] U
INNER JOIN [dbo].[OrganizationUserView] OU ON U.[Id] = OU.[UserId]
INNER JOIN [dbo].[OrganizationView] O ON OU.[OrganizationId] = O.[Id]
INNER JOIN [dbo].[OrganizationDomainView] OD ON OU.[OrganizationId] = OD.[OrganizationId]
WHERE U.[Id] = @UserId
AND (
OU.[Status] = 2 -- Confirmed
OR OU.[Status] = -1 -- Revoked
)
AND OD.[VerifiedDate] IS NOT NULL
AND U.[Email] LIKE '%@' + OD.[DomainName];
END
GO
Loading