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

[AC-292] Public Api - allow configuration of custom permissions #4022

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
b183325
Recombine InviteUser overloads in OrganizationService
eliykat Apr 25, 2024
82fb3f8
Refactor InviteUserAsync to take an object
eliykat Apr 26, 2024
4e65cb9
Add Custom Permissions to Public API
eliykat Apr 26, 2024
fa2aae2
dotnet format
eliykat Apr 26, 2024
32a3525
Set default permissions object
eliykat Apr 26, 2024
b6a1952
Merge branch 'main' into ac/ac-292/public-api---configure-custom-perm…
eliykat Apr 26, 2024
855a91d
Merge branch 'main' into ac/ac-292/public-api---configure-custom-perm…
eliykat Apr 28, 2024
d42b5f0
Merge remote-tracking branch 'origin/main' into ac/ac-292/public-api-…
eliykat May 6, 2024
1d17a18
Enable nullable
eliykat May 6, 2024
3c62d6b
Fix test names to use proper sut method name
eliykat May 6, 2024
e444dde
Add tests
eliykat May 6, 2024
baf44ae
Fix deserialization issue for PermissionsModel
eliykat May 7, 2024
bad71ba
Merge remote-tracking branch 'origin/main' into ac/ac-292/public-api-…
eliykat May 7, 2024
9c9f8f7
Update comment
eliykat May 7, 2024
7115338
Update comment
eliykat May 7, 2024
ce13267
dotnet format
eliykat May 7, 2024
bafdb8e
Add validation
eliykat May 7, 2024
1c90693
Merge branch 'main' into ac/ac-292/public-api---configure-custom-perm…
eliykat May 9, 2024
012e708
Merge branch 'main' into ac/ac-292/public-api---configure-custom-perm…
eliykat May 10, 2024
125bb4a
dotnet format
eliykat May 10, 2024
9e964f5
Undo billing changes
eliykat May 13, 2024
ca74e90
Merge remote-tracking branch 'origin/main' into ac/ac-292/public-api-…
eliykat May 20, 2024
b1891f4
Bootstrap public api integration tests
eliykat May 20, 2024
7d41711
Stub out TODOs
eliykat May 20, 2024
dd66448
Remove unnecessary assert helper
eliykat May 20, 2024
05784da
Use Substitute instead of NoopPaymentService
eliykat May 20, 2024
8ebe305
Add integration tests
eliykat May 21, 2024
6bc9c89
dotnet format
eliykat May 21, 2024
c47bc27
Merge remote-tracking branch 'origin/main' into ac/ac-292/public-api-…
eliykat May 21, 2024
a467fc6
Merge remote-tracking branch 'origin/main' into ac/ac-292/public-api-…
eliykat May 22, 2024
8a26d6a
Fix custom user with deprecated permissions logic
eliykat May 22, 2024
4fd6713
Merge remote-tracking branch 'origin/main' into ac/ac-292/public-api-…
eliykat May 24, 2024
117bd3a
Merge branch 'main' into ac/ac-292/public-api---configure-custom-perm…
eliykat May 27, 2024
5c1a5d3
Merge branch 'main' into ac/ac-292/public-api---configure-custom-perm…
eliykat May 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ private static string GetEnumDisplayName(Enum value)
]
: Array.Empty<CollectionAccessSelection>();

await _organizationService.InviteUsersAsync(organization.Id, user.Id,
await _organizationService.InviteUsersAsync(organization.Id, user.Id, systemUser: null,
new (OrganizationUserInvite, string)[]
{
(
Expand Down
61 changes: 60 additions & 1 deletion bitwarden_license/src/Scim/Models/ScimUserRequestModel.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,67 @@
namespace Bit.Scim.Models;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.Enums;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data;
using Bit.Core.Utilities;

namespace Bit.Scim.Models;

public class ScimUserRequestModel : BaseScimUserModel
{
public ScimUserRequestModel()
: base(false)
{ }

public OrganizationUserInvite ToOrganizationUserInvite(ScimProviderType scimProvider)
{
return new OrganizationUserInvite
{
Emails = new[] { EmailForInvite(scimProvider) },

// Permissions cannot be set via SCIM so we use default values
Type = OrganizationUserType.User,
AccessAll = false,
Collections = new List<CollectionAccessSelection>(),
Groups = new List<Guid>()
};
}

private string EmailForInvite(ScimProviderType scimProvider)
{
var email = PrimaryEmail?.ToLowerInvariant();

if (!string.IsNullOrWhiteSpace(email))
{
return email;
}

switch (scimProvider)
{
case ScimProviderType.AzureAd:
return UserName?.ToLowerInvariant();
default:
email = WorkEmail?.ToLowerInvariant();
if (string.IsNullOrWhiteSpace(email))
{
email = Emails?.FirstOrDefault()?.Value?.ToLowerInvariant();
}

return email;
}
}

public string ExternalIdForInvite()
{
if (!string.IsNullOrWhiteSpace(ExternalId))
{
return ExternalId;
}

if (!string.IsNullOrWhiteSpace(UserName))
{
return UserName;
}

return CoreHelpers.RandomString(15);
}
}
45 changes: 8 additions & 37 deletions bitwarden_license/src/Scim/Users/PostUserCommand.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
using Bit.Core.AdminConsole.Enums;
using Bit.Core.Enums;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Utilities;
using Bit.Scim.Context;
using Bit.Scim.Models;
using Bit.Scim.Users.Interfaces;
Expand All @@ -30,23 +27,11 @@ public class PostUserCommand : IPostUserCommand

public async Task<OrganizationUserUserDetails> PostUserAsync(Guid organizationId, ScimUserRequestModel model)
{
var email = model.PrimaryEmail?.ToLowerInvariant();
if (string.IsNullOrWhiteSpace(email))
{
switch (_scimContext.RequestScimProvider)
{
case ScimProviderType.AzureAd:
email = model.UserName?.ToLowerInvariant();
break;
default:
email = model.WorkEmail?.ToLowerInvariant();
if (string.IsNullOrWhiteSpace(email))
{
email = model.Emails?.FirstOrDefault()?.Value?.ToLowerInvariant();
}
break;
}
}
var scimProvider = _scimContext.RequestScimProvider;
var invite = model.ToOrganizationUserInvite(scimProvider);

var email = invite.Emails.Single();
var externalId = model.ExternalIdForInvite();

if (string.IsNullOrWhiteSpace(email) || !model.Active)
{
Expand All @@ -60,28 +45,14 @@ public async Task<OrganizationUserUserDetails> PostUserAsync(Guid organizationId
throw new ConflictException();
}

string externalId = null;
if (!string.IsNullOrWhiteSpace(model.ExternalId))
{
externalId = model.ExternalId;
}
else if (!string.IsNullOrWhiteSpace(model.UserName))
{
externalId = model.UserName;
}
else
{
externalId = CoreHelpers.RandomString(15);
}

var orgUserByExternalId = orgUsers.FirstOrDefault(ou => ou.ExternalId == externalId);
if (orgUserByExternalId != null)
{
throw new ConflictException();
}

var invitedOrgUser = await _organizationService.InviteUserAsync(organizationId, EventSystemUser.SCIM, email,
OrganizationUserType.User, false, externalId, new List<CollectionAccessSelection>(), new List<Guid>());
var invitedOrgUser = await _organizationService.InviteUserAsync(organizationId, invitingUserId: null, EventSystemUser.SCIM,
invite, externalId);
var orgUser = await _organizationUserRepository.GetDetailsByIdAsync(invitedOrgUser.Id);

return orgUser;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ await sutProvider.GetDependency<IEventService>()
.Received().LogProviderOrganizationEventAsync(providerOrganization,
EventType.ProviderOrganization_Created);
await sutProvider.GetDependency<IOrganizationService>()
.Received().InviteUsersAsync(organization.Id, user.Id, Arg.Is<IEnumerable<(OrganizationUserInvite, string)>>(
.Received().InviteUsersAsync(organization.Id, user.Id, systemUser: null, Arg.Is<IEnumerable<(OrganizationUserInvite, string)>>(
t => t.Count() == 1 &&
t.First().Item1.Emails.Count() == 1 &&
t.First().Item1.Emails.First() == clientOwnerEmail &&
Expand Down Expand Up @@ -709,6 +709,7 @@ await sutProvider.GetDependency<IOrganizationService>()
.InviteUsersAsync(
organization.Id,
user.Id,
systemUser: null,
Arg.Is<IEnumerable<(OrganizationUserInvite, string)>>(
t =>
t.Count() == 1 &&
Expand Down Expand Up @@ -740,7 +741,7 @@ await sutProvider.GetDependency<IEventService>()
.Received().LogProviderOrganizationEventAsync(providerOrganization,
EventType.ProviderOrganization_Created);
await sutProvider.GetDependency<IOrganizationService>()
.Received().InviteUsersAsync(organization.Id, user.Id, Arg.Is<IEnumerable<(OrganizationUserInvite, string)>>(
.Received().InviteUsersAsync(organization.Id, user.Id, systemUser: null, Arg.Is<IEnumerable<(OrganizationUserInvite, string)>>(
t => t.Count() == 1 &&
t.First().Item1.Emails.Count() == 1 &&
t.First().Item1.Emails.First() == clientOwnerEmail &&
Expand Down
22 changes: 16 additions & 6 deletions bitwarden_license/test/Scim.Test/Users/PostUserCommandTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Repositories;
using Bit.Core.Services;
Expand Down Expand Up @@ -34,15 +34,25 @@ public async Task PostUser_Success(SutProvider<PostUserCommand> sutProvider, str
.Returns(organizationUsers);

sutProvider.GetDependency<IOrganizationService>()
.InviteUserAsync(organizationId, EventSystemUser.SCIM, scimUserRequestModel.PrimaryEmail.ToLowerInvariant(),
OrganizationUserType.User, false, externalId, Arg.Any<List<CollectionAccessSelection>>(),
Arg.Any<List<Guid>>())
.InviteUserAsync(organizationId, invitingUserId: null, EventSystemUser.SCIM,
Arg.Is<OrganizationUserInvite>(i =>
i.Emails.Single().Equals(scimUserRequestModel.PrimaryEmail.ToLowerInvariant()) &&
i.Type == OrganizationUserType.User &&
!i.AccessAll &&
!i.Collections.Any() &&
!i.Groups.Any()), externalId)
.Returns(newUser);

var user = await sutProvider.Sut.PostUserAsync(organizationId, scimUserRequestModel);

await sutProvider.GetDependency<IOrganizationService>().Received(1).InviteUserAsync(organizationId, EventSystemUser.SCIM, scimUserRequestModel.PrimaryEmail.ToLowerInvariant(),
OrganizationUserType.User, false, scimUserRequestModel.ExternalId, Arg.Any<List<CollectionAccessSelection>>(), Arg.Any<List<Guid>>());
await sutProvider.GetDependency<IOrganizationService>().Received(1).InviteUserAsync(organizationId,
invitingUserId: null, EventSystemUser.SCIM,
Arg.Is<OrganizationUserInvite>(i =>
i.Emails.Single().Equals(scimUserRequestModel.PrimaryEmail.ToLowerInvariant()) &&
i.Type == OrganizationUserType.User &&
!i.AccessAll &&
!i.Collections.Any() &&
!i.Groups.Any()), externalId);
await sutProvider.GetDependency<IOrganizationUserRepository>().Received(1).GetDetailsByIdAsync(newUser.Id);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public async Task Invite(Guid orgId, [FromBody] OrganizationUserInviteRequestMod
}

var userId = _userService.GetProperUserId(User);
await _organizationService.InviteUsersAsync(orgId, userId.Value,
await _organizationService.InviteUsersAsync(orgId, userId.Value, systemUser: null,
new (OrganizationUserInvite, string)[] { (new OrganizationUserInvite(model.ToData()), null) });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,11 @@
public async Task<IActionResult> Post([FromBody] MemberCreateRequestModel model)
{
var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(_currentContext.OrganizationId.Value);
var associations = model.Collections?.Select(c => c.ToCollectionAccessSelection(flexibleCollectionsIsEnabled)).ToList();
var invite = model.ToOrganizationUserInvite(flexibleCollectionsIsEnabled);

Check warning on line 130 in src/Api/AdminConsole/Public/Controllers/MembersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Controllers/MembersController.cs#L130

Added line #L130 was not covered by tests

var user = await _organizationService.InviteUserAsync(_currentContext.OrganizationId.Value, null,
model.Email, model.Type.Value, model.AccessAll.Value, model.ExternalId, associations, model.Groups);
var response = new MemberResponseModel(user, associations, flexibleCollectionsIsEnabled);
systemUser: null, invite, model.ExternalId);
var response = new MemberResponseModel(user, invite.Collections, flexibleCollectionsIsEnabled);

Check warning on line 134 in src/Api/AdminConsole/Public/Controllers/MembersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Controllers/MembersController.cs#L133-L134

Added lines #L133 - L134 were not covered by tests
return new JsonResult(response);
}

Expand Down
19 changes: 18 additions & 1 deletion src/Api/AdminConsole/Public/Models/MemberBaseModel.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.ComponentModel.DataAnnotations;
#nullable enable

using System.ComponentModel.DataAnnotations;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data;
Expand All @@ -21,6 +23,11 @@
AccessAll = user.AccessAll;
ExternalId = user.ExternalId;
ResetPasswordEnrolled = user.ResetPasswordKey != null;

if (user.Type == OrganizationUserType.Custom)
{
Permissions = new PermissionsModel(user.GetPermissions());
}

Check warning on line 30 in src/Api/AdminConsole/Public/Models/MemberBaseModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/MemberBaseModel.cs#L28-L30

Added lines #L28 - L30 were not covered by tests
}

public MemberBaseModel(OrganizationUserUserDetails user, bool flexibleCollectionsEnabled)
Expand All @@ -34,6 +41,11 @@
AccessAll = user.AccessAll;
ExternalId = user.ExternalId;
ResetPasswordEnrolled = user.ResetPasswordKey != null;

if (user.Type == OrganizationUserType.Custom)
{
Permissions = new PermissionsModel(user.GetPermissions());
}

Check warning on line 48 in src/Api/AdminConsole/Public/Models/MemberBaseModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/MemberBaseModel.cs#L46-L48

Added lines #L46 - L48 were not covered by tests
}

/// <summary>
Expand All @@ -59,6 +71,11 @@
/// </summary>
[Required]
public bool ResetPasswordEnrolled { get; set; }
/// <summary>
/// The member's custom permissions if the member has a Custom role. If not supplied, all custom permissions will
/// default to false.
/// </summary>
public PermissionsModel? Permissions { get; set; }

Check warning on line 78 in src/Api/AdminConsole/Public/Models/MemberBaseModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/MemberBaseModel.cs#L78

Added line #L78 was not covered by tests

// TODO: AC-2188 - Remove this method when the custom users with no other permissions than 'Edit/Delete Assigned Collections' are migrated
private OrganizationUserType GetFlexibleCollectionsUserType(OrganizationUserType type, Permissions permissions)
Expand Down
67 changes: 67 additions & 0 deletions src/Api/AdminConsole/Public/Models/PermissionsModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#nullable enable

using System.Text.Json.Serialization;
using Bit.Core.Models.Data;

namespace Bit.Api.AdminConsole.Public.Models;

/// <summary>
/// Represents a member's custom permissions if the member has a Custom role.
/// </summary>
public class PermissionsModel
{
[JsonConstructor]
public PermissionsModel() { }
public PermissionsModel(Permissions? data)
{

Check warning on line 16 in src/Api/AdminConsole/Public/Models/PermissionsModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/PermissionsModel.cs#L14-L16

Added lines #L14 - L16 were not covered by tests
if (data is null)
{
return;

Check warning on line 19 in src/Api/AdminConsole/Public/Models/PermissionsModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/PermissionsModel.cs#L18-L19

Added lines #L18 - L19 were not covered by tests
}

AccessEventLogs = data.AccessEventLogs;
AccessImportExport = data.AccessImportExport;
AccessReports = data.AccessReports;
CreateNewCollections = data.CreateNewCollections;
EditAnyCollection = data.EditAnyCollection;
DeleteAnyCollection = data.DeleteAnyCollection;
ManageGroups = data.ManageGroups;
ManagePolicies = data.ManagePolicies;
ManageSso = data.ManageSso;
ManageUsers = data.ManageUsers;
ManageResetPassword = data.ManageResetPassword;
ManageScim = data.ManageScim;
}

Check warning on line 34 in src/Api/AdminConsole/Public/Models/PermissionsModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/PermissionsModel.cs#L22-L34

Added lines #L22 - L34 were not covered by tests

public bool AccessEventLogs { get; set; }
public bool AccessImportExport { get; set; }
public bool AccessReports { get; set; }
public bool CreateNewCollections { get; set; }
public bool EditAnyCollection { get; set; }
public bool DeleteAnyCollection { get; set; }
public bool ManageGroups { get; set; }
public bool ManagePolicies { get; set; }
public bool ManageSso { get; set; }
public bool ManageUsers { get; set; }
public bool ManageResetPassword { get; set; }
public bool ManageScim { get; set; }

Check warning on line 47 in src/Api/AdminConsole/Public/Models/PermissionsModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/PermissionsModel.cs#L36-L47

Added lines #L36 - L47 were not covered by tests

public Permissions ToData()
{
return new Permissions
{
AccessEventLogs = AccessEventLogs,
AccessImportExport = AccessImportExport,
AccessReports = AccessReports,
CreateNewCollections = CreateNewCollections,
EditAnyCollection = EditAnyCollection,
DeleteAnyCollection = DeleteAnyCollection,
ManageGroups = ManageGroups,
ManagePolicies = ManagePolicies,
ManageSso = ManageSso,
ManageUsers = ManageUsers,
ManageResetPassword = ManageResetPassword,
ManageScim = ManageScim
};
}

Check warning on line 66 in src/Api/AdminConsole/Public/Models/PermissionsModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/PermissionsModel.cs#L50-L66

Added lines #L50 - L66 were not covered by tests
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.ComponentModel.DataAnnotations;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Business;
using Bit.Core.Utilities;

namespace Bit.Api.AdminConsole.Public.Models.Request;
Expand All @@ -19,4 +21,24 @@
{
throw new NotImplementedException();
}

public OrganizationUserInvite ToOrganizationUserInvite(bool flexibleCollectionsIsEnabled)
{

Check warning on line 26 in src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs#L26

Added line #L26 was not covered by tests
var invite = new OrganizationUserInvite
{
Emails = new[] { Email },
Type = Type.Value,
AccessAll = AccessAll.Value,
Collections = Collections?.Select(c => c.ToCollectionAccessSelection(flexibleCollectionsIsEnabled)).ToList(),
Groups = Groups
};

Check warning on line 34 in src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs#L28-L34

Added lines #L28 - L34 were not covered by tests

// Permissions property is optional for backwards compatibility with existing usage
if (Type is OrganizationUserType.Custom && Permissions is not null)
{
invite.Permissions = Permissions.ToData();
}

Check warning on line 40 in src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs#L38-L40

Added lines #L38 - L40 were not covered by tests

return invite;
}

Check warning on line 43 in src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/AdminConsole/Public/Models/Request/MemberCreateRequestModel.cs#L42-L43

Added lines #L42 - L43 were not covered by tests
}