Merge pull request #354 from DigitalExcellence/bugfix/352-dataofficer…

…-institution-update Bugfix/352 dataofficer institution update
DigitalExcellence · Jan 18, 2021 · a534f58 · a534f58
2 parents 6ba7f07 + 4081c9a
commit a534f58
Show file tree

Hide file tree

Showing 5 changed files with 357 additions and 42 deletions.
diff --git a/API/Common/AuthorizationHelper.cs b/API/Common/AuthorizationHelper.cs
@@ -54,12 +54,28 @@ public async Task<bool> UserIsAllowed(User loggedInUser, string scope, string da
         {
             bool hasUserWriteScope = userService.UserHasScope(loggedInUser.IdentityId, scope);
             bool hasCorrectDataOfficerRights =
-                userService.UserHasScope(loggedInUser.IdentityId, dataOfficerScope) &&
-                await userService.HasSameInstitution(loggedInUser.Id, propertyOfUserId);
+                await SameInstitutionAndInstitutionScope(loggedInUser, dataOfficerScope, propertyOfUserId);
             bool isAllowed = hasUserWriteScope || hasCorrectDataOfficerRights;
             return isAllowed;
         }
 
+        /// <summary>
+        /// This method checks if a user has the same institution, and both should not have null. It
+        /// also checks if the user has the correct institution scope that allows changes in the
+        /// same institution.
+        /// </summary>
+        /// <param name="loggedInUser">The user model of the logged in user.</param>
+        /// <param name="institutionScope">The required scope for accessing this
+        /// endpoint for data officers within the same institution.</param>
+        /// <param name="propertyOfUserId">The id of the user owner of the property
+        /// which the logged in user wants to access.</param>
+        /// <returns>Bool: true if the user is allowed, false if the user is not allowed.</returns>
+        public async Task<bool> SameInstitutionAndInstitutionScope(User loggedInUser, string institutionScope, int propertyOfUserId)
+        {
+            return userService.UserHasScope(loggedInUser.IdentityId, institutionScope) &&
+                   await userService.HasSameInstitution(loggedInUser.Id, propertyOfUserId);
+        }
+
     }
 
 }
diff --git a/API/Common/IAuthorizationHelper.cs b/API/Common/IAuthorizationHelper.cs
@@ -42,6 +42,21 @@ public Task<bool> UserIsAllowed(User loggedInUser,
                                               string dataOfficerScope,
                                               int propertyOfUserId);
 
+        /// <summary>
+        /// This method checks if a user has the same institution, and both should not have null. It
+        /// also checks if the user has the correct institution scope that allows changes in the
+        /// same institution.
+        /// </summary>
+        /// <param name="loggedInUser">The user model of the logged in user.</param>
+        /// <param name="institutionScope">The required scope for accessing this
+        /// endpoint for data officers within the same institution.</param>
+        /// <param name="propertyOfUserId">The id of the user owner of the property
+        /// which the logged in user wants to access.</param>
+        /// <returns>Bool: true if the user is allowed, false if the user is not allowed.</returns>
+        Task<bool> SameInstitutionAndInstitutionScope(User loggedInUser,
+                                                      string institutionScope,
+                                                      int propertyOfUserId);
+
     }
 
 }
diff --git a/API/Controllers/UserController.cs b/API/Controllers/UserController.cs
@@ -259,22 +259,8 @@ public async Task<IActionResult> UpdateAccount(int userId, [FromBody] UserResour
                 }
             }
 
-            User currentUser = await HttpContext.GetContextUser(userService).ConfigureAwait(false);
-            bool isAllowed = userService.UserHasScope(currentUser.IdentityId, nameof(Defaults.Scopes.UserWrite));
-
-            if(currentUser.Id != userId && !isAllowed)
-            {
-                ProblemDetails problem = new ProblemDetails
-                 {
-                     Title = "Failed to edit the user.",
-                     Detail = "The user is not allowed to edit this user.",
-                     Instance = "E28BEBC0-AE7C-49F5-BDDC-3C13972B75D0"
-                 };
-                return Unauthorized(problem);
-            }
-
-            User user = await userService.FindAsync(userId);
-            if(user == null)
+            User userToUpdate = await userService.FindAsync(userId);
+            if(userToUpdate == null)
             {
                 ProblemDetails problem = new ProblemDetails
                 {
@@ -285,12 +271,53 @@ public async Task<IActionResult> UpdateAccount(int userId, [FromBody] UserResour
                 return NotFound(problem);
             }
 
-            mapper.Map(userResource, user);
+            User currentUser = await HttpContext.GetContextUser(userService)
+                                                .ConfigureAwait(false);
+            bool hasFullAllowance = userService.UserHasScope(currentUser.IdentityId, nameof(Defaults.Scopes.UserWrite));
+
+            // Has institution excluded allowance if it's your own account or if the user has the right scope for the same institution.
+            // In the last case, the institution has to be the same.
+            bool hasInstitutionExcludedAllowance = currentUser.Id == userId ||
+                                                   await authorizationHelper.SameInstitutionAndInstitutionScope(currentUser,
+                                                                    nameof(Defaults.Scopes.InstitutionUserWrite), userToUpdate.Id);
 
-            userService.Update(user);
+            if(!hasFullAllowance &&
+               !hasInstitutionExcludedAllowance)
+            {
+                ProblemDetails problem = new ProblemDetails
+                {
+                    Title = "Failed to edit the user.",
+                    Detail = "The user is not allowed to edit this user.",
+                    Instance = "E28BEBC0-AE7C-49F5-BDDC-3C13972B75D0"
+                };
+                return Unauthorized(problem);
+            }
+
+            // Roles that have the institution excluded allowance or it's own account can update everything except the institution id.
+            if(hasInstitutionExcludedAllowance)
+            {
+                // Check if no institution is specified, and if an institution is specified, this institution can't be
+                // updated. However, the institution can be null, because the data officer has enough rights to delete
+                // a user from their institution.
+                if(userResource.InstitutionId != null &&
+                   userResource.InstitutionId != userToUpdate.InstitutionId)
+                {
+                    ProblemDetails problem = new ProblemDetails
+                    {
+                        Title = "Failed to edit the user",
+                        Detail = "The user has not enough rights to update the institution id",
+                        Instance = "DD72C521-1D06-4E11-A0E0-AAE515E7F900"
+                    };
+                    return Unauthorized(problem);
+                }
+            }
+
+            mapper.Map(userResource, userToUpdate);
+
+            userService.Update(userToUpdate);
             userService.Save();
 
-            return Ok(mapper.Map<User, UserResourceResult>(user));
+            return Ok(mapper.Map<User, UserResourceResult>(userToUpdate));
         }
 
         /// <summary>

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Refactored Postman tests - [#328](https://github.com/DigitalExcellence/dex-backend/issues/328)
+- Resolve update institution id for data officer bug - [#352](https://github.com/DigitalExcellence/dex-backend/issues/352)
 
 ### Security