Skip to content

Commit

Permalink
fix(appRoles): don't allowed to upload duplicate app roles (#877)
Browse files Browse the repository at this point in the history 
- eliminate duplicate role values from api: /api/apps/appreleaseprocess/{appId}/role & /api/apps/AppChange/{appId}/role/activeapp

[Ref: #868](#868)

---------

Co-authored-by: Norbert Truchsess <[email protected]>
  • Loading branch information
@tfjanjua @ntruchsess
tfjanjua and ntruchsess authored Aug 14, 2024
1 parent 8cc86ed commit 11390e5
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/administration/Administration.Service/BusinessLogic/UserRolesBusinessLogic.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private async Task<IEnumerable<UserRoleWithId>> ModifyUserRolesInternal(
var distinctRoles = roles.Where(role => !string.IsNullOrWhiteSpace(role)).Distinct().ToImmutableList();
var existingRoles = await getUserRoleModificationData(companyUserId, distinctRoles, offerId).ToListAsync().ConfigureAwait(false);
existingRoles.DuplicatesBy(x => x.CompanyUserRoleText).IfAny(
duplicateRoles => throw new ConflictException($"roles {string.Join(",", $"{duplicateRoles.Select(role => $"[{role.CompanyUserRoleText}, {role.CompanyUserRoleId}]")}")} are ambigous"));
duplicateRoles => throw new ConflictException($"roles {string.Join(",", duplicateRoles.Select(role => $"[{role.CompanyUserRoleText}, {role.CompanyUserRoleId}]"))} are ambigous"));

distinctRoles.Except(existingRoles.Where(r => r.IsAssignable).Select(r => r.CompanyUserRoleText)).IfAny(
nonExistingRoles => throw new ControllerArgumentException($"Invalid roles {string.Join(",", nonExistingRoles)}", nameof(roles)));
Expand Down
12 changes: 10 additions & 2 deletions src/marketplace/Apps.Service/BusinessLogic/AppChangeBusinessLogic.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,19 @@ private async Task<IEnumerable<AppRoleData>> InsertActiveAppUserRoleAsync(Guid a
{
throw new ForbiddenException($"Company {_identityData.CompanyId} is not the provider company of app {appId}");
}
var roleData = await AppExtensions.CreateUserRolesWithDescriptions(_portalRepositories.GetInstance<IUserRolesRepository>(), appId, userRoles);

// When user will try to upload the same role names which are already attched to an APP.
// so, no role will be added against the given appId so, no need to procced further
// No need to Add roles to client
// No need to update the Offer entity
// When nothing has happened, no need to send notifications
if (!roleData.Any())
return roleData;

var roleData = AppExtensions.CreateUserRolesWithDescriptions(_portalRepositories.GetInstance<IUserRolesRepository>(), appId, userRoles);
foreach (var clientId in result.ClientClientIds)
{
await _provisioningManager.AddRolesToClientAsync(clientId, userRoles.Select(x => x.Role)).ConfigureAwait(ConfigureAwaitOptions.None);
await _provisioningManager.AddRolesToClientAsync(clientId, roleData.Select(x => x.RoleName)).ConfigureAwait(ConfigureAwaitOptions.None);
}

_portalRepositories.GetInstance<IOfferRepository>().AttachAndModifyOffer(appId, offer =>
Expand Down
8 changes: 7 additions & 1 deletion src/marketplace/Apps.Service/BusinessLogic/AppReleaseBusinessLogic.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,13 @@ private async Task<IEnumerable<AppRoleData>> InsertAppUserRoleAsync(Guid appId,
{
throw new ForbiddenException($"Company {companyId} is not the provider company of app {appId}");
}
var roleData = AppExtensions.CreateUserRolesWithDescriptions(_portalRepositories.GetInstance<IUserRolesRepository>(), appId, userRoles);
var roleData = await AppExtensions.CreateUserRolesWithDescriptions(_portalRepositories.GetInstance<IUserRolesRepository>(), appId, userRoles);

// When user will try to upload the same role names which are already attched to an APP.
// so, no role will be added against the given appId so, no need to procced further
// No need to update the Offer entity
if (!roleData.Any())
return roleData;

_portalRepositories.GetInstance<IOfferRepository>().AttachAndModifyOffer(appId, offer =>
offer.DateLastChanged = DateTimeOffset.UtcNow);
Expand Down
23 changes: 21 additions & 2 deletions src/marketplace/Apps.Service/Extensions/AppExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

using Org.Eclipse.TractusX.Portal.Backend.Apps.Service.ViewModels;
using Org.Eclipse.TractusX.Portal.Backend.Framework.ErrorHandling;
using Org.Eclipse.TractusX.Portal.Backend.Framework.Linq;
using Org.Eclipse.TractusX.Portal.Backend.PortalBackend.DBAccess.Repositories;
using Org.Eclipse.TractusX.Portal.Backend.PortalBackend.PortalEntities.Entities;

Expand Down Expand Up @@ -46,6 +47,7 @@ public static void ValidateAppUserRole(Guid appId, IEnumerable<AppUserRole> appU
{
throw new ControllerArgumentException("Language Code must not be empty");
}
appUserRolesDescription.DuplicatesBy(x => x.Role).IfAny(duplicateRoles => throw new ControllerArgumentException($"Roles are ambiguous: {string.Join(",", duplicateRoles.Select(x => x.Role))}"));
}

/// <summary>
Expand All @@ -56,13 +58,30 @@ public static void ValidateAppUserRole(Guid appId, IEnumerable<AppUserRole> appU
/// <param name="appId">id of the app to create the roles for</param>
/// <param name="userRoles">the user roles to add</param>
/// <returns>returns the created appRoleData</returns>
public static IEnumerable<AppRoleData> CreateUserRolesWithDescriptions(IUserRolesRepository userRolesRepository, Guid appId, IEnumerable<AppUserRole> userRoles) =>
userRoles.Zip(
public static async Task<IEnumerable<AppRoleData>> CreateUserRolesWithDescriptions(IUserRolesRepository userRolesRepository, Guid appId, IEnumerable<AppUserRole> userRoles)
{
userRoles = await GetUniqueAppUserRoles(userRolesRepository, appId, userRoles);
return userRoles.Zip(
userRolesRepository.CreateAppUserRoles(userRoles.Select(x => (appId, x.Role))),
(AppUserRole appUserRole, UserRole userRole) =>
{
userRolesRepository.CreateAppUserRoleDescriptions(appUserRole.Descriptions.Select(appUserRoleDescription => (userRole.Id, appUserRoleDescription.LanguageCode, appUserRoleDescription.Description)));
return new AppRoleData(userRole.Id, appUserRole.Role);
})
.ToList();
}

/// <summary>
/// Get unique roles by eleminating the duplicate roles from the request (client) and existing roles from the Database
/// </summary>
/// <remarks></remarks>
/// <param name="userRolesRepository">repository</param>
/// <param name="appId">id of the app</param>
/// <param name="userRoles">the app user roles</param>
/// <returns>returns the filtered and unique roles</returns>
private static async Task<IEnumerable<AppUserRole>> GetUniqueAppUserRoles(IUserRolesRepository userRolesRepository, Guid appId, IEnumerable<AppUserRole> userRoles)
{
var existingRoles = await userRolesRepository.GetUserRolesForOfferIdAsync(appId).ToListAsync().ConfigureAwait(false);
return userRoles.ExceptBy(existingRoles, userRole => userRole.Role);
}
}
86 changes: 86 additions & 0 deletions tests/marketplace/Apps.Service.Tests/BusinessLogic/AppChangeBusinessLogicTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ public async Task AddActiveAppUserRoleAsync_ExecutesSuccessfully()
A.CallTo(() => _portalRepositories.GetInstance<IOfferRepository>().GetInsertActiveAppUserRoleDataAsync(appId, OfferTypeId.APP))
.Returns((true, appName, _identity.CompanyId, clientIds));

A.CallTo(() => _userRolesRepository.GetUserRolesForOfferIdAsync(appId))
.Returns(new[] { "Exsiting Role" }.ToAsyncEnumerable());

IEnumerable<UserRole>? userRoles = null;
A.CallTo(() => _userRolesRepository.CreateAppUserRoles(A<IEnumerable<(Guid, string)>>._))
.ReturnsLazily((IEnumerable<(Guid AppId, string Role)> appRoles) =>
Expand Down Expand Up @@ -226,6 +229,89 @@ public async Task AddActiveAppUserRoleAsync_ExecutesSuccessfully()
);
}

[Fact]
public async Task AddActiveAppUserRoleAsync_WithExistingData_NoRowEffecting()
{
//Arrange
var appId = _fixture.Create<Guid>();
var appName = _fixture.Create<string>();
var appAssignedRoleDesc = _fixture.CreateMany<string>(3).Select(role => new AppUserRole(role, _fixture.CreateMany<AppUserRoleDescription>(2).ToImmutableArray())).ToImmutableArray();
var clientIds = new[] { "client" };

A.CallTo(() => _portalRepositories.GetInstance<IOfferRepository>().GetInsertActiveAppUserRoleDataAsync(appId, OfferTypeId.APP))
.Returns((true, appName, _identity.CompanyId, clientIds));

A.CallTo(() => _userRolesRepository.GetUserRolesForOfferIdAsync(appId))
.Returns(appAssignedRoleDesc.Select(x => x.Role).ToAsyncEnumerable());

IEnumerable<UserRole>? userRoles = null;
A.CallTo(() => _userRolesRepository.CreateAppUserRoles(A<IEnumerable<(Guid, string)>>._))
.ReturnsLazily((IEnumerable<(Guid AppId, string Role)> appRoles) =>
{
userRoles = appRoles.Select(x => new UserRole(Guid.NewGuid(), x.Role, x.AppId)).ToImmutableArray();
return userRoles;
});

var userRoleDescriptions = new List<IEnumerable<UserRoleDescription>>();
A.CallTo(() => _userRolesRepository.CreateAppUserRoleDescriptions(A<IEnumerable<(Guid, string, string)>>._))
.ReturnsLazily((IEnumerable<(Guid RoleId, string LanguageCode, string Description)> roleLanguageDescriptions) =>
{
var createdUserRoleDescriptions = roleLanguageDescriptions.Select(x => new UserRoleDescription(x.RoleId, x.LanguageCode, x.Description)).ToImmutableArray();
userRoleDescriptions.Add(createdUserRoleDescriptions);
return createdUserRoleDescriptions;
});
var existingOffer = _fixture.Create<Offer>();
existingOffer.DateLastChanged = DateTimeOffset.UtcNow;
A.CallTo(() => _offerRepository.AttachAndModifyOffer(appId, A<Action<Offer>>._, A<Action<Offer>?>._))
.Invokes((Guid _, Action<Offer> setOptionalParameters, Action<Offer>? initializeParemeters) =>
{
initializeParemeters?.Invoke(existingOffer);
setOptionalParameters(existingOffer);
});
A.CallTo(() => _notificationService.CreateNotifications(A<IEnumerable<UserRoleConfig>>._, A<Guid>._, A<IEnumerable<(string? content, NotificationTypeId notificationTypeId)>>._, A<Guid>._, A<bool?>._))
.Returns(_fixture.CreateMany<Guid>(4).AsFakeIAsyncEnumerable(out var createNotificationsResultAsyncEnumerator));

//Act
var result = await _sut.AddActiveAppUserRoleAsync(appId, appAssignedRoleDesc);

//Assert
A.CallTo(() => _offerRepository.GetInsertActiveAppUserRoleDataAsync(appId, OfferTypeId.APP)).MustHaveHappened();

A.CallTo(() => _userRolesRepository.CreateAppUserRoles(A<IEnumerable<(Guid, string)>>._)).MustHaveHappenedOnceExactly();
userRoles.Should().NotBeNull()
.And.HaveCount(0);

A.CallTo(() => _userRolesRepository.CreateAppUserRoleDescriptions(A<IEnumerable<(Guid, string, string)>>._)).MustNotHaveHappened();
userRoleDescriptions.Should().NotBeNull()
.And.HaveCount(0);
A.CallTo(() => _offerRepository.AttachAndModifyOffer(appId, A<Action<Offer>>._, A<Action<Offer>?>._)).MustNotHaveHappened();
A.CallTo(() => _notificationService.CreateNotifications(A<IEnumerable<UserRoleConfig>>._, A<Guid>._, A<IEnumerable<(string? content, NotificationTypeId notificationTypeId)>>._, A<Guid>._, A<bool?>._))
.MustNotHaveHappened();
A.CallTo(() => createNotificationsResultAsyncEnumerator.MoveNextAsync())
.MustNotHaveHappened();
A.CallTo(() => _provisioningManager.AddRolesToClientAsync("client", A<IEnumerable<string>>.That.IsSameSequenceAs(appAssignedRoleDesc.Select(x => x.Role))))
.MustNotHaveHappened();

result.Should().NotBeNull()
.And.HaveCount(0);
}

[Fact]
public async Task AddDuplicateActiveAppUserRoleAsync_ThrowsControllerArgumentException()
{
//Arrange
var appId = _fixture.Create<Guid>();
var roleId = _fixture.Create<string>();
var appAssignedRoleDesc = _fixture.CreateMany<string>(3).Select(role => new AppUserRole(roleId, _fixture.CreateMany<AppUserRoleDescription>(2).ToImmutableArray())).ToImmutableArray();

//Act
async Task Act() => await _sut.AddActiveAppUserRoleAsync(appId, appAssignedRoleDesc);

//Assert
var error = await Assert.ThrowsAsync<ControllerArgumentException>(Act);
error.Message.Should().Be($"Roles are ambiguous: {roleId},{roleId}");
}

[Fact]
public async Task AddActiveAppUserRoleAsync_WithCompanyUserIdNotSet_ThrowsForbiddenException()
{
Expand Down
75 changes: 75 additions & 0 deletions tests/marketplace/Apps.Service.Tests/BusinessLogic/AppReleaseBusinessLogicTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ public async Task CreateServiceOffering_WithValidDataAndEmptyDescriptions_Return
A.CallTo(() => _offerRepository.IsProviderCompanyUserAsync(appId, _identity.CompanyId, OfferTypeId.APP))
.Returns((true, true));

A.CallTo(() => _userRolesRepository.GetUserRolesForOfferIdAsync(appId))
.Returns(new[] { "Exsiting Role" }.ToAsyncEnumerable());

IEnumerable<UserRole>? userRoles = null;
A.CallTo(() => _userRolesRepository.CreateAppUserRoles(A<IEnumerable<(Guid, string)>>._))
.ReturnsLazily((IEnumerable<(Guid AppId, string Role)> appRoles) =>
Expand Down Expand Up @@ -214,6 +217,78 @@ public async Task CreateServiceOffering_WithValidDataAndEmptyDescriptions_Return
);
}

[Fact]
public async Task CreateServiceOffering_WithExistingData_NoRowEffecting()
{
// Arrange
var appId = _fixture.Create<Guid>();
var appUserRoles = _fixture.CreateMany<string>(3).Select(role => new AppUserRole(role, _fixture.CreateMany<AppUserRoleDescription>(2).ToImmutableArray())).ToImmutableArray();

A.CallTo(() => _offerRepository.IsProviderCompanyUserAsync(appId, _identity.CompanyId, OfferTypeId.APP))
.Returns((true, true));

A.CallTo(() => _userRolesRepository.GetUserRolesForOfferIdAsync(appId))
.Returns(appUserRoles.Select(x => x.Role).ToAsyncEnumerable());

IEnumerable<UserRole>? userRoles = null;
A.CallTo(() => _userRolesRepository.CreateAppUserRoles(A<IEnumerable<(Guid, string)>>._))
.ReturnsLazily((IEnumerable<(Guid AppId, string Role)> appRoles) =>
{
userRoles = appRoles.Select(x => new UserRole(Guid.NewGuid(), x.Role, x.AppId)).ToImmutableArray();
return userRoles;
});

var userRoleDescriptions = new List<IEnumerable<UserRoleDescription>>();
A.CallTo(() => _userRolesRepository.CreateAppUserRoleDescriptions(A<IEnumerable<(Guid, string, string)>>._))
.ReturnsLazily((IEnumerable<(Guid RoleId, string LanguageCode, string Description)> roleLanguageDescriptions) =>
{
var createdUserRoleDescriptions = roleLanguageDescriptions.Select(x => new UserRoleDescription(x.RoleId, x.LanguageCode, x.Description)).ToImmutableArray();
userRoleDescriptions.Add(createdUserRoleDescriptions);
return createdUserRoleDescriptions;
});
var existingOffer = _fixture.Create<Offer>();
existingOffer.DateLastChanged = DateTimeOffset.UtcNow;
A.CallTo(() => _offerRepository.AttachAndModifyOffer(appId, A<Action<Offer>>._, A<Action<Offer>?>._))
.Invokes((Guid _, Action<Offer> setOptionalParameters, Action<Offer>? initializeParemeters) =>
{
initializeParemeters?.Invoke(existingOffer);
setOptionalParameters(existingOffer);
});
// Act
var result = await _sut.AddAppUserRoleAsync(appId, appUserRoles);

// Assert
A.CallTo(() => _offerRepository.IsProviderCompanyUserAsync(A<Guid>._, A<Guid>._, A<OfferTypeId>._)).MustHaveHappened();

A.CallTo(() => _userRolesRepository.CreateAppUserRoles(A<IEnumerable<(Guid, string)>>._)).MustHaveHappenedOnceExactly();
userRoles.Should().NotBeNull()
.And.HaveCount(0);

A.CallTo(() => _offerRepository.AttachAndModifyOffer(appId, A<Action<Offer>>._, A<Action<Offer>?>._)).MustNotHaveHappened();
A.CallTo(() => _userRolesRepository.CreateAppUserRoleDescriptions(A<IEnumerable<(Guid, string, string)>>._)).MustNotHaveHappened();
userRoleDescriptions.Should().NotBeNull()
.And.HaveCount(0);

result.Should().NotBeNull()
.And.HaveCount(0);
}

[Fact]
public async Task CreateServiceOffering_WithValidButDuplicateData_ThrowsControllerArgumentException()
{
// Arrange
var appId = _fixture.Create<Guid>();
var roleId = _fixture.Create<string>();
var appUserRoles = _fixture.CreateMany<string>(3).Select(role => new AppUserRole(roleId, _fixture.CreateMany<AppUserRoleDescription>(2).ToImmutableArray())).ToImmutableArray();

// Act
async Task Act() => await _sut.AddAppUserRoleAsync(appId, appUserRoles);

// Assert
var error = await Assert.ThrowsAsync<ControllerArgumentException>(Act);
error.Message.Should().Be($"Roles are ambiguous: {roleId},{roleId}");
}

#region AddAppAsync

[Fact]
Expand Down

0 comments on commit 11390e5

Please sign in to comment.