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-13298] Modify members access logic #18

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
97 changes: 53 additions & 44 deletions src/Api/Tools/Controllers/ReportsController.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
๏ปฟusing Bit.Api.Tools.Models.Response;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces;
using Bit.Core.Context;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Vault.Queries;
using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests;
using Bit.Core.Tools.Models.Data;
using Bit.Core.Tools.ReportFeatures.OrganizationReportMembers.Interfaces;
using Bit.Core.Tools.ReportFeatures.Requests;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;

Expand All @@ -17,33 +13,49 @@ namespace Bit.Api.Tools.Controllers;
[Authorize("Application")]
public class ReportsController : Controller
{
private readonly IOrganizationUserUserDetailsQuery _organizationUserUserDetailsQuery;
private readonly IGroupRepository _groupRepository;
private readonly ICollectionRepository _collectionRepository;
private readonly ICurrentContext _currentContext;
private readonly IOrganizationCiphersQuery _organizationCiphersQuery;
private readonly IApplicationCacheService _applicationCacheService;
private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery;
private readonly IMemberAccessCipherDetailsQuery _memberAccessCipherDetailsQuery;

public ReportsController(
IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery,
IGroupRepository groupRepository,
ICollectionRepository collectionRepository,
ICurrentContext currentContext,
IOrganizationCiphersQuery organizationCiphersQuery,
IApplicationCacheService applicationCacheService,
ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery
IMemberAccessCipherDetailsQuery memberAccessCipherDetailsQuery
)
{
_organizationUserUserDetailsQuery = organizationUserUserDetailsQuery;
_groupRepository = groupRepository;
_collectionRepository = collectionRepository;
_currentContext = currentContext;
_organizationCiphersQuery = organizationCiphersQuery;
_applicationCacheService = applicationCacheService;
_twoFactorIsEnabledQuery = twoFactorIsEnabledQuery;
_memberAccessCipherDetailsQuery = memberAccessCipherDetailsQuery;
}

/// <summary>
/// Organization member information containing a list of cipher ids
/// assigned
/// </summary>
/// <param name="orgId">Organzation Id</param>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Typo in 'Organzation', should be 'Organization'

/// <returns>IEnumerable of MemberCipherDetailsResponseModel</returns>
/// <exception cref="NotFoundException">If Access reports permission is not assigned</exception>
[HttpGet("member-cipher-details/{orgId}")]
public async Task<IEnumerable<MemberCipherDetailsResponseModel>> GetMemberCipherDetails(Guid orgId)
{
// Using the AccessReports permission here until new permissions
// are needed for more control over reports
if (!await _currentContext.AccessReports(orgId))
{
throw new NotFoundException();
}
Comment on lines +40 to +43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using a more specific exception like UnauthorizedException instead of NotFoundException for permission issues


var memberCipherDetails = await GetMemberCipherDetails(new MemberAccessCipherDetailsRequest { OrganizationId = orgId });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Method name clash with the public method. Rename private method to avoid confusion


var responses = memberCipherDetails.Select(x => new MemberCipherDetailsResponseModel(x));

return responses;
}

/// <summary>
/// Access details for an organization member. Includes the member information,
/// group collection assignment, and item counts
/// </summary>
/// <param name="orgId">Organization Id</param>
/// <returns>IEnumerable of MemberAccessReportResponseModel</returns>
/// <exception cref="NotFoundException">If Access reports permission is not assigned</exception>
[HttpGet("member-access/{orgId}")]
public async Task<IEnumerable<MemberAccessReportResponseModel>> GetMemberAccessReport(Guid orgId)
{
Expand All @@ -52,26 +64,23 @@ public async Task<IEnumerable<MemberAccessReportResponseModel>> GetMemberAccessR
throw new NotFoundException();
}

var orgUsers = await _organizationUserUserDetailsQuery.GetOrganizationUserUserDetails(
new OrganizationUserUserDetailsQueryRequest
{
OrganizationId = orgId,
IncludeCollections = true,
IncludeGroups = true
});
var memberCipherDetails = await GetMemberCipherDetails(new MemberAccessCipherDetailsRequest { OrganizationId = orgId });

var orgGroups = await _groupRepository.GetManyByOrganizationIdAsync(orgId);
var orgAbility = await _applicationCacheService.GetOrganizationAbilityAsync(orgId);
var orgCollectionsWithAccess = await _collectionRepository.GetManyByOrganizationIdWithAccessAsync(orgId);
var orgItems = await _organizationCiphersQuery.GetAllOrganizationCiphers(orgId);
var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(orgUsers);
var responses = memberCipherDetails.Select(x => new MemberAccessReportResponseModel(x));

var reports = MemberAccessReportResponseModel.CreateReport(
orgGroups,
orgCollectionsWithAccess,
orgItems,
organizationUsersTwoFactorEnabled,
orgAbility);
return reports;
return responses;
}

/// <summary>
/// Contains the organization member info, the cipher ids associated with the member,
/// and details on their collections, groups, and permissions
/// </summary>
/// <param name="request">Request to the MemberAccessCipherDetailsQuery</param>
/// <returns>IEnumerable of MemberAccessCipherDetails</returns>
private async Task<IEnumerable<MemberAccessCipherDetails>> GetMemberCipherDetails(MemberAccessCipherDetailsRequest request)
{
var memberCipherDetails =
await _memberAccessCipherDetailsQuery.GetMemberAccessCipherDetails(request);
return memberCipherDetails;
}
Comment on lines +80 to 85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: This private method could be inlined as it's only called once and doesn't add much abstraction

}
162 changes: 11 additions & 151 deletions src/Api/Tools/Models/Response/MemberAccessReportModel.cs
Original file line number Diff line number Diff line change
@@ -1,30 +1,7 @@
๏ปฟusing Bit.Core.AdminConsole.Entities;
using Bit.Core.Entities;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Vault.Models.Data;
๏ปฟusing Bit.Core.Tools.Models.Data;

namespace Bit.Api.Tools.Models.Response;

/// <summary>
/// Member access details. The individual item for the detailed member access
/// report. A collection can be assigned directly to a user without a group or
/// the user can be assigned to a collection through a group. Group level permissions
/// can override collection level permissions.
/// </summary>
public class MemberAccessReportAccessDetails
{
public Guid? CollectionId { get; set; }
public Guid? GroupId { get; set; }
public string GroupName { get; set; }
public string CollectionName { get; set; }
public int ItemCount { get; set; }
public bool? ReadOnly { get; set; }
public bool? HidePasswords { get; set; }
public bool? Manage { get; set; }
}

/// <summary>
/// Contains the collections and group collections a user has access to including
/// the permission level for the collection and group collection.
Expand All @@ -40,134 +17,17 @@ public class MemberAccessReportResponseModel
public int TotalItemCount { get; set; }
public Guid? UserGuid { get; set; }
public bool UsesKeyConnector { get; set; }
public IEnumerable<MemberAccessReportAccessDetails> AccessDetails { get; set; }
Comment on lines 18 to 19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: UserGuid is not being set in the constructor

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: UsesKeyConnector is not being set in the constructor

public IEnumerable<MemberAccessDetails> AccessDetails { get; set; }

/// <summary>
/// Generates a report for all members of an organization. Containing summary information
/// such as item, collection, and group counts. As well as detailed information on the
/// user and group collections along with their permissions
/// </summary>
/// <param name="orgGroups">Organization groups collection</param>
/// <param name="orgCollectionsWithAccess">Collections for the organization and the groups/users and permissions</param>
/// <param name="orgItems">Cipher items for the organization with the collections associated with them</param>
/// <param name="organizationUsersTwoFactorEnabled">Organization users and two factor status</param>
/// <param name="orgAbility">Organization ability for account recovery status</param>
/// <returns>List of the MemberAccessReportResponseModel</returns>;
public static IEnumerable<MemberAccessReportResponseModel> CreateReport(
ICollection<Group> orgGroups,
ICollection<Tuple<Collection, CollectionAccessDetails>> orgCollectionsWithAccess,
IEnumerable<CipherOrganizationDetailsWithCollections> orgItems,
IEnumerable<(OrganizationUserUserDetails user, bool twoFactorIsEnabled)> organizationUsersTwoFactorEnabled,
OrganizationAbility orgAbility)
public MemberAccessReportResponseModel(MemberAccessCipherDetails memberAccessCipherDetails)
{
var orgUsers = organizationUsersTwoFactorEnabled.Select(x => x.user);
// Create a dictionary to lookup the group names later.
var groupNameDictionary = orgGroups.ToDictionary(x => x.Id, x => x.Name);

// Get collections grouped and into a dictionary for counts
var collectionItems = orgItems
.SelectMany(x => x.CollectionIds,
(x, b) => new { CipherId = x.Id, CollectionId = b })
.GroupBy(y => y.CollectionId,
(key, g) => new { CollectionId = key, Ciphers = g });
var collectionItemCounts = collectionItems.ToDictionary(x => x.CollectionId, x => x.Ciphers.Count());


// Loop through the org users and populate report and access data
var memberAccessReport = new List<MemberAccessReportResponseModel>();
foreach (var user in orgUsers)
{
// Take the collections/groups and create the access details items
var groupAccessDetails = new List<MemberAccessReportAccessDetails>();
var userCollectionAccessDetails = new List<MemberAccessReportAccessDetails>();
foreach (var tCollect in orgCollectionsWithAccess)
{
var itemCounts = collectionItemCounts.TryGetValue(tCollect.Item1.Id, out var itemCount) ? itemCount : 0;
if (tCollect.Item2.Groups.Count() > 0)
{
var groupDetails = tCollect.Item2.Groups.Where((tCollectGroups) => user.Groups.Contains(tCollectGroups.Id)).Select(x =>
new MemberAccessReportAccessDetails
{
CollectionId = tCollect.Item1.Id,
CollectionName = tCollect.Item1.Name,
GroupId = x.Id,
GroupName = groupNameDictionary[x.Id],
ReadOnly = x.ReadOnly,
HidePasswords = x.HidePasswords,
Manage = x.Manage,
ItemCount = itemCounts,
});
groupAccessDetails.AddRange(groupDetails);
}

// All collections assigned to users and their permissions
if (tCollect.Item2.Users.Count() > 0)
{
var userCollectionDetails = tCollect.Item2.Users.Where((tCollectUser) => tCollectUser.Id == user.Id).Select(x =>
new MemberAccessReportAccessDetails
{
CollectionId = tCollect.Item1.Id,
CollectionName = tCollect.Item1.Name,
ReadOnly = x.ReadOnly,
HidePasswords = x.HidePasswords,
Manage = x.Manage,
ItemCount = itemCounts,
});
userCollectionAccessDetails.AddRange(userCollectionDetails);
}
}

var report = new MemberAccessReportResponseModel
{
UserName = user.Name,
Email = user.Email,
TwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == user.Id).twoFactorIsEnabled,
// Both the user's ResetPasswordKey must be set and the organization can UseResetPassword
AccountRecoveryEnabled = !string.IsNullOrEmpty(user.ResetPasswordKey) && orgAbility.UseResetPassword,
UserGuid = user.Id,
UsesKeyConnector = user.UsesKeyConnector
};

var userAccessDetails = new List<MemberAccessReportAccessDetails>();
if (user.Groups.Any())
{
var userGroups = groupAccessDetails.Where(x => user.Groups.Contains(x.GroupId.GetValueOrDefault()));
userAccessDetails.AddRange(userGroups);
}

// There can be edge cases where groups don't have a collection
var groupsWithoutCollections = user.Groups.Where(x => !userAccessDetails.Any(y => x == y.GroupId));
if (groupsWithoutCollections.Count() > 0)
{
var emptyGroups = groupsWithoutCollections.Select(x => new MemberAccessReportAccessDetails
{
GroupId = x,
GroupName = groupNameDictionary[x],
ItemCount = 0
});
userAccessDetails.AddRange(emptyGroups);
}

if (user.Collections.Any())
{
var userCollections = userCollectionAccessDetails.Where(x => user.Collections.Any(y => x.CollectionId == y.Id));
userAccessDetails.AddRange(userCollections);
}
report.AccessDetails = userAccessDetails;

report.TotalItemCount = collectionItems
.Where(x => report.AccessDetails.Any(y => x.CollectionId == y.CollectionId))
.SelectMany(x => x.Ciphers)
.GroupBy(g => g.CipherId).Select(grp => grp.FirstOrDefault())
.Count();

// Distinct items only
var distinctItems = report.AccessDetails.Where(x => x.CollectionId.HasValue).Select(x => x.CollectionId).Distinct();
report.CollectionsCount = distinctItems.Count();
report.GroupsCount = report.AccessDetails.Select(x => x.GroupId).Where(y => y.HasValue).Distinct().Count();
memberAccessReport.Add(report);
}
return memberAccessReport;
this.UserName = memberAccessCipherDetails.UserName;
this.Email = memberAccessCipherDetails.Email;
this.TwoFactorEnabled = memberAccessCipherDetails.TwoFactorEnabled;
this.AccountRecoveryEnabled = memberAccessCipherDetails.AccountRecoveryEnabled;
this.GroupsCount = memberAccessCipherDetails.GroupsCount;
this.CollectionsCount = memberAccessCipherDetails.CollectionsCount;
this.TotalItemCount = memberAccessCipherDetails.TotalItemCount;
this.AccessDetails = memberAccessCipherDetails.AccessDetails;
}

}
24 changes: 24 additions & 0 deletions src/Api/Tools/Models/Response/MemberCipherDetailsResponseModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
๏ปฟusing Bit.Core.Tools.Models.Data;

namespace Bit.Api.Tools.Models.Response;

public class MemberCipherDetailsResponseModel
{
public string UserName { get; set; }
public string Email { get; set; }
public bool UsesKeyConnector { get; set; }
Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider making properties init-only to promote immutability


/// <summary>
/// A distinct list of the cipher ids associated with
/// the organization member
/// </summary>
public IEnumerable<string> CipherIds { get; set; }

public MemberCipherDetailsResponseModel(MemberAccessCipherDetails memberAccessCipherDetails)
{
this.UserName = memberAccessCipherDetails.UserName;
this.Email = memberAccessCipherDetails.Email;
this.UsesKeyConnector = memberAccessCipherDetails.UsesKeyConnector;
this.CipherIds = memberAccessCipherDetails.CipherIds;
Comment on lines +19 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Remove 'this.' keyword for consistency with C# conventions

}
Comment on lines +17 to +23
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Add null check for memberAccessCipherDetails parameter

}
43 changes: 43 additions & 0 deletions src/Core/Tools/Models/Data/MemberAccessCipherDetails.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
๏ปฟnamespace Bit.Core.Tools.Models.Data;

public class MemberAccessDetails
{
public Guid? CollectionId { get; set; }
public Guid? GroupId { get; set; }
Comment on lines +5 to +6
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using non-nullable Guid for CollectionId and GroupId if they are always expected to have a value

public string GroupName { get; set; }
public string CollectionName { get; set; }
Comment on lines +7 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Add null checks or use nullable string types for GroupName and CollectionName

public int ItemCount { get; set; }
public bool? ReadOnly { get; set; }
public bool? HidePasswords { get; set; }
public bool? Manage { get; set; }
Comment on lines +10 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using non-nullable bool for ReadOnly, HidePasswords, and Manage if they always have a value


/// <summary>
/// The CipherIds associated with the group/collection access
/// </summary>
public IEnumerable<string> CollectionCipherIds { get; set; }
}

public class MemberAccessCipherDetails
{
public string UserName { get; set; }
public string Email { get; set; }
Comment on lines +22 to +23
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Add null checks or use nullable string types for UserName and Email

public bool TwoFactorEnabled { get; set; }
public bool AccountRecoveryEnabled { get; set; }
public int GroupsCount { get; set; }
public int CollectionsCount { get; set; }
public int TotalItemCount { get; set; }
public Guid? UserGuid { get; set; }
public bool UsesKeyConnector { get; set; }

/// <summary>
/// The details for the member's collection access depending
/// on the collections and groups they are assigned to
/// </summary>
public IEnumerable<MemberAccessDetails> AccessDetails { get; set; }

/// <summary>
/// A distinct list of the cipher ids associated with
/// the organization member
/// </summary>
public IEnumerable<string> CipherIds { get; set; }
}
Loading
Loading