Allowing admins to change passwords for other user accounts

CHANGELOG.md

@@ -3,6 +3,7 @@
 
 ## [22.05.5] - 2022/06/28
 * [UI]: Fixed bug preventing export of project samples table due to invalid url. [PR 1331](https://github.com/phac-nml/irida/pull/1331)
+* [UI]: Updated user account security page to allow admins to change passwords for other users. See [PR 1330](https://github.com/phac-nml/irida/pull/1330)
 
 ## [22.05.4] - 2022/06/16
 * [UI]: Fixed bug preventing filter samples by file to fail on invalid url. See [PR 1318](https://github.com/phac-nml/irida/pull/1318)

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/services/UIUsersService.java

@@ -137,13 +137,13 @@ public UserDetailsResponse getUser(Long userId, Boolean mailFailure, Principal p
 		boolean isAdmin = RoleUtilities.isAdmin(principalUser);
 		boolean canEditUserInfo = canEditUserInfo(principalUser, user);
 		boolean canEditUserStatus = canEditUserStatus(principalUser, user);
-		boolean canChangePassword = canChangePassword(principalUser, user);
+		boolean isOwnAccount = isOwnAccount(principalUser, user);
 		boolean canCreatePasswordReset = canCreatePasswordReset(principalUser, user);
 
 		String currentRoleName = messageSource.getMessage("systemRole." + user.getSystemRole().getName(), null, locale);
 
 		return new UserDetailsResponse(userDetails, currentRoleName, mailConfigured, mailFailure, isAdmin,
-				canEditUserInfo, canEditUserStatus, canChangePassword, canCreatePasswordReset);
+				canEditUserInfo, canEditUserStatus, isOwnAccount, canCreatePasswordReset);
 	}
 
 	/**
@@ -222,27 +222,49 @@ public UserDetailsResponse updateUser(Long userId, UserEditRequest userEditReque
 	 */
 	public UserDetailsResponse changeUserPassword(Long userId, String oldPassword, String newPassword,
 			Principal principal, HttpServletRequest request) {
+		User user = userService.read(userId);
 		User principalUser = userService.getUserByUsername(principal.getName());
+		boolean principalAdmin = principalUser.getAuthorities().contains(Role.ROLE_ADMIN);
+		boolean usersEqual = user.equals(principalUser);
 		Map<String, Object> updatedValues = new HashMap<>();
 		Map<String, String> errors = new HashMap<>();
 
-		if (!Strings.isNullOrEmpty(oldPassword) || !Strings.isNullOrEmpty(newPassword)) {
-			if (!passwordEncoder.matches(oldPassword, principalUser.getPassword())) {
+		if (usersEqual) {
+			//check both oldPassword & newPassword exist if a user is updating their own password
+			if (Strings.isNullOrEmpty(oldPassword)) {
 				errors.put("oldPassword",
-						messageSource.getMessage("server.user.edit.password.old.incorrect", null, request.getLocale()));
+						messageSource.getMessage("server.user.edit.password.old.required", null, request.getLocale()));
+			} else if (Strings.isNullOrEmpty(newPassword)) {
+				errors.put("newPassword",
+						messageSource.getMessage("server.user.edit.password.new.required", null, request.getLocale()));
 			} else {
-				updatedValues.put("password", newPassword);
+				if (!passwordEncoder.matches(oldPassword, principalUser.getPassword())) {
+					errors.put("oldPassword", messageSource.getMessage("server.user.edit.password.old.incorrect", null,
+							request.getLocale()));
+				} else {
+					updatedValues.put("password", newPassword);
+				}
+			}
+		} else {
+			//only check newPassword exists if an admin is updating another user's password
+			if (principalAdmin) {
+				if (Strings.isNullOrEmpty(newPassword)) {
+					errors.put("newPassword", messageSource.getMessage("server.user.edit.password.new.required", null,
+							request.getLocale()));
+				} else {
+					updatedValues.put("password", newPassword);
+				}
 			}
 		}
 
 		if (errors.isEmpty()) {
 			try {
-				User user = userService.updateFields(userId, updatedValues);
+				User updatedUser = userService.updateFields(userId, updatedValues);
 
 				// If the user is updating their account make sure you update it in the session variable
-				if (user != null && principal.getName().equals(user.getUsername())) {
+				if (updatedUser != null && usersEqual) {
 					HttpSession session = request.getSession();
-					session.setAttribute(UserSecurityInterceptor.CURRENT_USER_DETAILS, user);
+					session.setAttribute(UserSecurityInterceptor.CURRENT_USER_DETAILS, updatedUser);
 				}
 			} catch (ConstraintViolationException | DataIntegrityViolationException | PasswordReusedException ex) {
 				errors = handleCreateUpdateException(ex, request.getLocale());
@@ -320,7 +342,7 @@ private Map<String, String> handleCreateUpdateException(Exception ex, Locale loc
 			EntityExistsException eex = (EntityExistsException) ex;
 			errors.put(eex.getFieldName(), eex.getMessage());
 		} else if (ex instanceof PasswordReusedException) {
-			errors.put("password", messageSource.getMessage("server.user.edit.passwordReused", null, locale));
+			errors.put("newPassword", messageSource.getMessage("server.user.edit.passwordReused", null, locale));
 		}
 
 		return errors;
@@ -355,15 +377,15 @@ private boolean canEditUserStatus(User principalUser, User user) {
 	}
 
 	/**
-	 * Check if the logged in user is allowed to change their password.
+	 * Check if the logged in user is modifying their own account.
 	 *
 	 * @param principalUser - the currently logged in principal
 	 * @param user          - the user to edit
 	 * @return boolean if the principal can change their password
+	 * @return if the user is an admin
 	 */
-	private boolean canChangePassword(User principalUser, User user) {
+	private boolean isOwnAccount(User principalUser, User user) {
 		boolean usersEqual = user.equals(principalUser);
-
 		return usersEqual;
 	}
 

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/users/UsersAjaxController.java

@@ -82,8 +82,7 @@ public ResponseEntity<Map<String, String>> updateUser(@PathVariable Long userId,
 		UserDetailsResponse response = uiUsersService.updateUser(userId, userEditRequest, principal, request);
 
 		if (response.hasErrors())
-			return ResponseEntity.status(HttpStatus.BAD_REQUEST)
-					.body(response.getErrors());
+			return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.getErrors());
 		else
 			return ResponseEntity.ok(null);
 
@@ -101,15 +100,14 @@ public ResponseEntity<Map<String, String>> updateUser(@PathVariable Long userId,
 	 */
 	@RequestMapping(value = "/{userId}/changePassword", method = RequestMethod.POST)
 	public ResponseEntity<Map<String, String>> changeUserPassword(@PathVariable Long userId,
-			@RequestParam String oldPassword, @RequestParam String newPassword, Principal principal,
+			@RequestParam(required = false) String oldPassword, @RequestParam String newPassword, Principal principal,
 			HttpServletRequest request) {
 
 		UserDetailsResponse response = uiUsersService.changeUserPassword(userId, oldPassword, newPassword, principal,
 				request);
 
 		if (response.hasErrors())
-			return ResponseEntity.status(HttpStatus.BAD_REQUEST)
-					.body(response.getErrors());
+			return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response.getErrors());
 		else
 			return ResponseEntity.ok(null);
 	}
@@ -142,7 +140,8 @@ public ResponseEntity<UserDetailsResponse> getUserDetails(@PathVariable("userId"
 	public ResponseEntity<AjaxResponse> adminNewPasswordReset(@PathVariable Long userId, Principal principal,
 			Locale locale) {
 		try {
-			return ResponseEntity.ok(new AjaxSuccessResponse(uiUsersService.adminNewPasswordReset(userId, principal, locale)));
+			return ResponseEntity.ok(
+					new AjaxSuccessResponse(uiUsersService.adminNewPasswordReset(userId, principal, locale)));
 		} catch (UIEmailSendException e) {
 			return ResponseEntity.status(HttpStatus.CONFLICT).body(new AjaxErrorResponse(e.getMessage()));
 		}

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/users/UsersController.java

@@ -86,7 +86,7 @@ public String getUsersPage() {
 	 * @return The name of the user account page
 	 */
 	@RequestMapping({ "/{userId}", "/{userId}/*" })
-	@PreAuthorize("hasAnyRole('ROLE_ADMIN', 'ROLE_MANAGER') or principal.id == #userId")
+	@PreAuthorize("hasAnyRole('ROLE_ADMIN') or principal.id == #userId")
 	public String getUserDetailsPage(@PathVariable Long userId) {
 		return SPECIFIC_USER_PAGE;
 	}

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/users/dto/UserDetailsResponse.java

@@ -17,21 +17,21 @@ public class UserDetailsResponse extends AjaxResponse {
 	private boolean isAdmin;
 	private boolean canEditUserInfo;
 	private boolean canEditUserStatus;
-	private boolean canChangePassword;
+	private boolean isOwnAccount;
 	private boolean canCreatePasswordReset;
 	private Map<String, String> errors;
 
 	public UserDetailsResponse(UserDetailsModel userDetails, String currentRole, boolean mailConfigured,
 			boolean mailFailure, boolean isAdmin, boolean canEditUserInfo, boolean canEditUserStatus,
-			boolean canChangePassword, boolean canCreatePasswordReset) {
+			boolean isOwnAccount, boolean canCreatePasswordReset) {
 		this.userDetails = userDetails;
 		this.currentRole = currentRole;
 		this.mailConfigured = mailConfigured;
 		this.mailFailure = mailFailure;
 		this.isAdmin = isAdmin;
 		this.canEditUserInfo = canEditUserInfo;
 		this.canEditUserStatus = canEditUserStatus;
-		this.canChangePassword = canChangePassword;
+		this.isOwnAccount = isOwnAccount;
 		this.canCreatePasswordReset = canCreatePasswordReset;
 	}
 
@@ -95,12 +95,12 @@ public void setCanEditUserStatus(boolean canEditUserStatus) {
 		this.canEditUserStatus = canEditUserStatus;
 	}
 
-	public boolean isCanChangePassword() {
-		return canChangePassword;
+	public boolean isOwnAccount() {
+		return isOwnAccount;
 	}
 
-	public void setCanChangePassword(boolean canChangePassword) {
-		this.canChangePassword = canChangePassword;
+	public void setOwnAccount(boolean isOwnAccount) {
+		this.isOwnAccount = isOwnAccount;
 	}
 
 	public boolean isCanCreatePasswordReset() {
@@ -137,14 +137,14 @@ public boolean equals(Object o) {
 		UserDetailsResponse that = (UserDetailsResponse) o;
 		return mailConfigured == that.mailConfigured && mailFailure == that.mailFailure && isAdmin == that.isAdmin
 				&& canEditUserInfo == that.canEditUserInfo && canEditUserStatus == that.canEditUserStatus
-				&& canChangePassword == that.canChangePassword && canCreatePasswordReset == that.canCreatePasswordReset
+				&& isOwnAccount == that.isOwnAccount && canCreatePasswordReset == that.canCreatePasswordReset
 				&& Objects.equals(userDetails, that.userDetails) && Objects.equals(currentRole, that.currentRole)
 				&& Objects.equals(errors, that.errors);
 	}
 
 	@Override
 	public int hashCode() {
 		return Objects.hash(userDetails, currentRole, mailConfigured, mailFailure, isAdmin, canEditUserInfo,
-				canEditUserStatus, canChangePassword, canCreatePasswordReset, errors);
+				canEditUserStatus, isOwnAccount, canCreatePasswordReset, errors);
 	}
 }

src/main/resources/i18n/messages.properties

@@ -516,7 +516,9 @@ user.create.username.taken=This username is already taken. Please try a differen
 user.create.email.taken=This email is already taken. Please supply a different email.
 user.create.phone.no-numbers=Phone number must contain numbers
 user.setpassword=Require User Activation
-server.user.edit.password.old.incorrect=The Old Password is incorrect
+server.user.edit.password.new.required=A new password is required
+server.user.edit.password.old.required=The old password is required
+server.user.edit.password.old.incorrect=The old password is incorrect
 server.user.edit.password.match=Passwords do not match.
 server.user.edit.emailConflict=The given email already exists in the system.
 server.user.edit.passwordReused=This password has already been used.  Enter a new password.

src/main/webapp/pages/template/base.html

@@ -19,5 +19,4 @@
 	<th:block th:utext="${analytics}"></th:block>
 </th:block>
 </body>
-</body>
 </html>

src/main/webapp/resources/js/apis/users/users.js

@@ -11,7 +11,7 @@ export const userApi = createApi({
   baseQuery: fetchBaseQuery({
     baseUrl: BASE_URL,
   }),
-  tagTypes: ["User"],
+  tagTypes: ["User", "PasswordReset"],
   endpoints: (build) => ({
     /*
     Get user details.

src/main/webapp/resources/js/components/UsersTable/UsersTable.jsx

@@ -2,16 +2,16 @@ import React, { useContext } from "react";
 import { PagedTable, PagedTableContext } from "../ant.design/PagedTable";
 import { useSetUsersDisabledStatusMutation } from "../../apis/users/users";
 import { Checkbox } from "antd";
-import { setBaseUrl } from "../../utilities/url-utilities";
 import { dateColumnFormat } from "../ant.design/table-renderers";
+import { setBaseUrl } from "../../utilities/url-utilities";
 
 /**
  * React component for displaying paged table of all users in the system
  * @returns {string|*}
  * @constructor
  */
 export function UsersTable() {
-  const {updateTable} = useContext(PagedTableContext);
+  const { updateTable } = useContext(PagedTableContext);
   const IS_ADMIN = window.TL._USER.systemRole === "ROLE_ADMIN";
   const [setUsersDisabledStatus] = useSetUsersDisabledStatusMutation();
 
@@ -54,10 +54,12 @@ export function UsersTable() {
       sorter: true,
       fixed: "left",
       render(text, full) {
-        return (
+        return IS_ADMIN ? (
           <a className="t-username" href={setBaseUrl(`users/${full.id}`)}>
             {text}
           </a>
+        ) : (
+          text
         );
       },
     },
@@ -102,13 +104,13 @@ export function UsersTable() {
       },
     },
     {
-      ...dateColumnFormat({className: "t-created"}),
+      ...dateColumnFormat({ className: "t-created" }),
       key: "createdDate",
       title: i18n("AdminUsersTable.created"),
       dataIndex: "createdDate",
     },
     {
-      ...dateColumnFormat({className: "t-modified"}),
+      ...dateColumnFormat({ className: "t-modified" }),
       key: "lastLogin",
       title: (
         <span className="t-modified-col">
@@ -122,7 +124,7 @@ export function UsersTable() {
   return (
     <PagedTable
       columns={columns}
-      onRow={(record) => (record.enabled ? {} : {className: "disabled"})}
+      onRow={(record) => (record.enabled ? {} : { className: "disabled" })}
     />
   );
 }

src/main/webapp/resources/js/pages/user/components/UserChangePasswordForm.jsx

@@ -15,10 +15,11 @@ import { SPACE_SM } from "../../../styles/spacing";
 /**
  * React component to display the user change password form.
  * @param {number} userId the identification number of the user
+ * @param {boolean} requireOldPassword whether to show the old password form field
  * @returns {*}
  * @constructor
  */
-export function UserChangePasswordForm({userId}) {
+export function UserChangePasswordForm({ userId, requireOldPassword }) {
   const [changeUserPassword] = useChangeUserPasswordMutation();
   const [form] = Form.useForm();
   const navigate = useNavigate();
@@ -32,7 +33,7 @@ export function UserChangePasswordForm({userId}) {
   ];
 
   const onFormFinish = (values) => {
-    changeUserPassword({userId: userId, ...values})
+    changeUserPassword({ userId: userId, ...values })
       .unwrap()
       .then(() => {
         notification.success({
@@ -56,7 +57,7 @@ export function UserChangePasswordForm({userId}) {
         {i18n("UserChangePasswordForm.title")}
       </Typography.Title>
       <Alert
-        style={{marginBottom: SPACE_SM}}
+        style={{ marginBottom: SPACE_SM }}
         message={i18n("UserChangePasswordForm.alert.title")}
         description={
           <Typography.Paragraph>
@@ -76,18 +77,20 @@ export function UserChangePasswordForm({userId}) {
         onFinish={onFormFinish}
         autoComplete="off"
       >
-        <Form.Item
-          label={i18n("UserChangePasswordForm.form.label.oldPassword")}
-          name="oldPassword"
-          rules={[
-            {
-              required: true,
-              message: i18n("UserChangePasswordForm.alert.rule1"),
-            },
-          ]}
-        >
-          <Input.Password/>
-        </Form.Item>
+        {requireOldPassword && (
+          <Form.Item
+            label={i18n("UserChangePasswordForm.form.label.oldPassword")}
+            name="oldPassword"
+            rules={[
+              {
+                required: true,
+                message: i18n("UserChangePasswordForm.alert.rule1"),
+              },
+            ]}
+          >
+            <Input.Password />
+          </Form.Item>
+        )}
         <Form.Item
           label={i18n("UserChangePasswordForm.form.label.newPassword")}
           name="newPassword"
@@ -96,7 +99,7 @@ export function UserChangePasswordForm({userId}) {
               required: true,
               message: i18n("UserChangePasswordForm.alert.rule1"),
             },
-            {min: 8, message: i18n("UserChangePasswordForm.alert.rule2")},
+            { min: 8, message: i18n("UserChangePasswordForm.alert.rule2") },
             {
               pattern: new RegExp("^.*[A-Z].*$"),
               message: i18n("UserChangePasswordForm.alert.rule3"),
@@ -115,7 +118,7 @@ export function UserChangePasswordForm({userId}) {
             },
           ]}
         >
-          <Input.Password/>
+          <Input.Password />
         </Form.Item>
         <Form.Item>
           <Button className="t-submit-btn" type="primary" htmlType="submit">