Fix for user assignment + add tests.
MODIFY
src/server/rest/v1/service/AuthorizationService.ts
src/server/rest/v1/service/AuthorizationService.ts
@@ -172,24 +172,39 @@ export default class AuthorizationService {
// Not an Admin?
if (userToken.role !== UserRole.ADMIN) {
if (Utils.isTenantComponentActive(tenant, TenantComponents.ORGANIZATION)) {
- // Get Site IDs from Site Admin flag
- const siteIDs = await AuthorizationService.getSiteAdminSiteIDs(tenant.id, userToken);
+ // Get Site IDs from Site Admin & Site Owner flag
+ const siteAdminSiteIDs = await AuthorizationService.getSiteAdminSiteIDs(tenant.id, userToken);
+ const siteOwnerSiteIDs: Array<string> = await AuthorizationService.getSiteOwnerSiteIDs(tenant.id, userToken);
+ const allSites: Array<string> = _.uniq([...siteAdminSiteIDs, ...siteOwnerSiteIDs]);
// Get User IDs from Site Admin flag
- if (!Utils.isEmptyArray(siteIDs)) {
+ if (!Utils.isEmptyArray(allSites)) {
// Check Site ID
- if (siteIDs.includes(filteredRequest.siteID)) {
+ if (allSites.includes(filteredRequest.siteID)) {
// Site Authorized, now check users
if (!Utils.isEmptyArray(filteredRequest.userIDs)) {
let foundInvalidUserID = false;
// Get authorized User IDs
const userIDs = await AuthorizationService.getAssignedUsersIDs(tenant.id, filteredRequest.siteID);
- // Check
+ switch (action) {
+ // Check if any of the users we want to assign are already assigned
+ case ServerAction.ADD_USERS_TO_SITE:
+ for (const userID of filteredRequest.userIDs) {
+ if (userIDs.includes(userID)) {
+ foundInvalidUserID = true;
+ break;
+ }
+ }
+ break;
+ default:
+ // Check if any of the users we want to unassign are already unassigned
for (const userID of filteredRequest.userIDs) {
if (!userIDs.includes(userID)) {
foundInvalidUserID = true;
break;
}
}
+ break;
+ }
if (!foundInvalidUserID) {
authorizationFilters.authorized = true;
}

MODIFY
test/api/SiteOrgTest.ts
test/api/SiteOrgTest.ts
@@ -306,11 +306,11 @@ describe('Site tests', function() {
it('Should not be able to update a site', async () => {
try {
// Change entity
- testData.newSite.name = 'New Name';
+ testData.createdSites[0].name = 'New Name';
// Update
await testData.userService.updateEntity(
testData.userService.siteApi,
- testData.newSite
+ testData.createdSites[0]
);
} catch (error) {
expect(error.actual).to.eq(403);
@@ -319,7 +319,7 @@ describe('Site tests', function() {
it('Should be able to update the site for which he is site admin', async () => {
// Change entity
- testData.newSite.name = 'New Name';
+ testData.createdSites[2].name = 'New Name';
// Update
await testData.userService.updateEntity(
testData.userService.siteApi,
@@ -329,7 +329,7 @@ describe('Site tests', function() {
it('Should be able to update the site for which he is site owner', async () => {
// Change entity
- testData.newSite.name = 'New Name';
+ testData.createdSites[3].name = 'New Name';
// Update
await testData.userService.updateEntity(
testData.userService.siteApi,
@@ -342,25 +342,23 @@ describe('Site tests', function() {
expect(assignmentResult.status).to.eq(403);
});
- // is failing
- // it('Should be able to assign a user to a site if he is site admin', async () => {
- // const assignmentResult = await assignUserToSite(ContextDefinition.USER_CONTEXTS.DEFAULT_ADMIN, testData.createdSites[2]);
- // expect(assignmentResult.status).to.eq(200);
- // const res = await testData.userService.siteApi.readUsersForSite(testData.newSite.id);
- // expect(res.data.count).to.eq(1);
- // const userContext = await testData.tenantContext.getUserContext(ContextDefinition.USER_CONTEXTS.BASIC_USER);
- // expect(res.data.result.map((site) => site.user.id)).to.contain(userContext.id);
- // });
-
- // is failing
- // it('Should be able to assign a user to a site if he is site owner', async () => {
- // const assignmentResult = await assignUserToSite(ContextDefinition.USER_CONTEXTS.DEFAULT_ADMIN, testData.createdSites[3]);
- // expect(assignmentResult.status).to.eq(200);
- // const res = await testData.userService.siteApi.readUsersForSite(testData.newSite.id);
- // expect(res.data.count).to.eq(1);
- // const userContext = await testData.tenantContext.getUserContext(ContextDefinition.USER_CONTEXTS.BASIC_USER);
- // expect(res.data.result.map((site) => site.user.id)).to.contain(userContext.id);
- // });
+ it('Should be able to assign a user to a site if he is site admin', async () => {
+ const assignmentResult = await assignUserToSite(ContextDefinition.USER_CONTEXTS.DEFAULT_ADMIN, testData.createdSites[2]);
+ expect(assignmentResult.status).to.eq(200);
+ const res = await testData.userService.siteApi.readUsersForSite(testData.createdSites[2].id);
+ expect(res.data.count).to.eq(2); // the site admin & the assigned user
+ const userContext = await testData.tenantContext.getUserContext(ContextDefinition.USER_CONTEXTS.BASIC_USER);
+ expect(res.data.result.map((site) => site.user.id)).to.contain(userContext.id);
+ });
+
+ it('Should be able to assign a user to a site if he is site owner', async () => {
+ const assignmentResult = await assignUserToSite(ContextDefinition.USER_CONTEXTS.DEFAULT_ADMIN, testData.createdSites[3]);
+ expect(assignmentResult.status).to.eq(200);
+ const res = await testData.userService.siteApi.readUsersForSite(testData.createdSites[3].id);
+ expect(res.data.count).to.eq(2); // the site owner & the assigned user
+ const userContext = await testData.tenantContext.getUserContext(ContextDefinition.USER_CONTEXTS.BASIC_USER);
+ expect(res.data.result.map((site) => site.user.id)).to.contain(userContext.id);
+ });
it('Should not be able to delete a site he is assigned to if he is not site admin or site owner', async () => {
try {

Fix for user assignment + add tests.