diff --git a/app/controllers/UserController.scala b/app/controllers/UserController.scala index 7074b9203ef..5a92da080f1 100755 --- a/app/controllers/UserController.scala +++ b/app/controllers/UserController.scala @@ -3,7 +3,6 @@ package controllers import play.silhouette.api.Silhouette import com.scalableminds.util.accesscontext.{DBAccessContext, GlobalAccessContext} import com.scalableminds.util.tools.{Fox, FoxImplicits} - import models.annotation.{AnnotationDAO, AnnotationService, AnnotationType} import models.organization.OrganizationService import models.team._ @@ -16,6 +15,10 @@ import com.scalableminds.util.objectid.ObjectId import javax.inject.Inject import models.user.Theme.Theme +import net.liftweb.common.{Failure, Full} +import play.silhouette.api.exceptions.ProviderException +import play.silhouette.api.util.Credentials +import play.silhouette.impl.providers.CredentialsProvider import security.WkEnv import scala.concurrent.ExecutionContext @@ -23,6 +26,7 @@ import scala.concurrent.ExecutionContext class UserController @Inject()(userService: UserService, userDAO: UserDAO, multiUserDAO: MultiUserDAO, + credentialsProvider: CredentialsProvider, organizationService: OrganizationService, annotationDAO: AnnotationDAO, teamMembershipService: TeamMembershipService, @@ -175,6 +179,7 @@ class UserController @Inject()(userService: UserService, ((__ \ "firstName").readNullable[String] and (__ \ "lastName").readNullable[String] and (__ \ "email").readNullable[String] and + (__ \ "password").readNullable[String] and (__ \ "isActive").readNullable[Boolean] and (__ \ "isAdmin").readNullable[Boolean] and (__ \ "isDatasetManager").readNullable[Boolean] and @@ -195,13 +200,45 @@ class UserController @Inject()(userService: UserService, Fox.successful(()) }) - private def checkAdminOnlyUpdates(user: User, - isActive: Boolean, - isAdmin: Boolean, - isDatasetManager: Boolean, - oldEmail: String, - email: String)(issuingUser: User): Boolean = - if (isActive && user.isAdmin == isAdmin && oldEmail == email && isDatasetManager == user.isDatasetManager) + private def checkTeamManagerOnlyUpdates(user: User, + experiences: Map[String, Int], + oldExperiences: Map[String, Int], + teams: List[TeamMembership], + oldTeams: List[TeamMembership])(issuingUser: User): Fox[Boolean] = + if (experiences == oldExperiences && teams == oldTeams) + Fox.successful(true) + else userService.isEditableBy(user, issuingUser) + + private def checkPasswordIfEmailChanged(user: User, passwordOpt: Option[String], oldEmail: String, email: String)( + issuingUser: User): Fox[Unit] = + if (oldEmail == email) { + Fox.successful(()) + } else if (user._id == issuingUser._id) { + passwordOpt match { + case Some(password) => + val credentials = Credentials(user._id.id, password) + Fox.fromFutureBox( + credentialsProvider + .authenticate(credentials) + .flatMap { loginInfo => + userService.retrieve(loginInfo).map { + case Some(_) => Full(()) + case None => Failure("error.noUser") + } + } + .recover { + case _: ProviderException => + Failure("user.email.change.passwordWrong") + }) + case None => Fox.failure("user.email.change.passwordWrong") + } + } else { + Fox.failure("notAllowed") + } + + private def checkAdminOnlyUpdates(user: User, isActive: Boolean, isAdmin: Boolean, isDatasetManager: Boolean)( + issuingUser: User): Boolean = + if (isActive && user.isAdmin == isAdmin && isDatasetManager == user.isDatasetManager) true else issuingUser.isAdminOf(user) @@ -224,16 +261,6 @@ class UserController @Inject()(userService: UserService, } yield () } else Fox.successful(()) - private def checkSuperUserOnlyUpdates(user: User, oldEmail: String, email: String)(issuingUser: User)( - implicit ctx: DBAccessContext): Fox[Unit] = - if (oldEmail == email) Fox.successful(()) - else - for { - count <- userDAO.countIdentitiesForMultiUser(user._multiUser) - issuingMultiUser <- multiUserDAO.findOne(issuingUser._multiUser) - _ <- Fox.fromBool(count <= 1 || issuingMultiUser.isSuperUser) ?~> "user.email.onlySuperUserCanChange" - } yield () - private def preventZeroAdmins(user: User, isAdmin: Boolean) = if (user.isAdmin && !isAdmin) { for { @@ -256,6 +283,7 @@ class UserController @Inject()(userService: UserService, case (firstNameOpt, lastNameOpt, emailOpt, + passwordOpt, isActiveOpt, isAdminOpt, isDatasetManagerOpt, @@ -264,6 +292,7 @@ class UserController @Inject()(userService: UserService, lastTaskTypeIdOpt) => for { user <- userDAO.findOne(userId) ?~> "user.notFound" ~> NOT_FOUND + // properties that can be changed by team managers and admins only: experiences, team memberships oldExperience <- userService.experiencesFor(user._id) oldAssignedMemberships <- userService.teamMembershipsFor(user._id) firstName = firstNameOpt.getOrElse(user.firstName) @@ -276,13 +305,19 @@ class UserController @Inject()(userService: UserService, assignedMemberships = assignedMembershipsOpt.getOrElse(oldAssignedMemberships) experiences = experiencesOpt.getOrElse(oldExperience) lastTaskTypeId = if (lastTaskTypeIdOpt.isEmpty) user.lastTaskTypeId.map(_.id) else lastTaskTypeIdOpt - _ <- Fox.assertTrue(userService.isEditableBy(user, request.identity)) ?~> "notAllowed" ~> FORBIDDEN - _ <- Fox.fromBool(checkAdminOnlyUpdates(user, isActive, isAdmin, isDatasetManager, oldEmail, email)( - issuingUser)) ?~> "notAllowed" ~> FORBIDDEN + _ <- Fox + .runIf(user._id != issuingUser._id)(Fox.assertTrue(userService.isEditableBy(user, request.identity))) ?~> "notAllowed" ~> FORBIDDEN + _ <- checkTeamManagerOnlyUpdates(user, + experiences, + oldExperience, + assignedMemberships, + oldAssignedMemberships)(issuingUser) ?~> "notAllowed" ~> FORBIDDEN + _ <- Fox + .fromBool(checkAdminOnlyUpdates(user, isActive, isAdmin, isDatasetManager)(issuingUser)) ?~> "notAllowed" ~> FORBIDDEN + _ <- checkPasswordIfEmailChanged(user, passwordOpt, oldEmail, email)(issuingUser) _ <- Fox.fromBool(checkNoSelfDeactivate(user, isActive)(issuingUser)) ?~> "user.noSelfDeactivate" ~> FORBIDDEN _ <- checkNoDeactivateWithRemainingTask(user, isActive) _ <- checkNoActivateBeyondLimit(user, isActive) - _ <- checkSuperUserOnlyUpdates(user, oldEmail, email)(issuingUser) _ <- preventZeroAdmins(user, isAdmin) _ <- preventZeroOwners(user, isActive) teams <- Fox.combined(assignedMemberships.map(t => diff --git a/app/models/user/EmailVerificationService.scala b/app/models/user/EmailVerificationService.scala index c15f965ba5f..5e5c839a43c 100644 --- a/app/models/user/EmailVerificationService.scala +++ b/app/models/user/EmailVerificationService.scala @@ -78,7 +78,7 @@ class EmailVerificationService @Inject()(conf: WkConf, ): Fox[Boolean] = for { multiUser: MultiUser <- multiUserDAO.findOne(user._multiUser) ?~> "user.notFound" - endOfGracePeriod: Instant = multiUser.created + conf.WebKnossos.User.EmailVerification.gracePeriod + endOfGracePeriod: Instant = multiUser.emailChangeDate + conf.WebKnossos.User.EmailVerification.gracePeriod overGracePeriod = endOfGracePeriod.isPast } yield !conf.WebKnossos.User.EmailVerification.required || multiUser.isEmailVerified || !overGracePeriod } diff --git a/app/models/user/MultiUser.scala b/app/models/user/MultiUser.scala index 0eb731d16d3..9c7fcdc9798 100644 --- a/app/models/user/MultiUser.scala +++ b/app/models/user/MultiUser.scala @@ -26,6 +26,7 @@ case class MultiUser( selectedTheme: Theme = Theme.auto, created: Instant = Instant.now, isEmailVerified: Boolean = false, + emailChangeDate: Instant = Instant.now, isDeleted: Boolean = false ) @@ -57,6 +58,7 @@ class MultiUserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext theme, Instant.fromSql(r.created), r.isemailverified, + Instant.fromSql(r.emailchangedate), r.isdeleted ) } @@ -91,7 +93,8 @@ class MultiUserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext _ <- run(q"""UPDATE webknossos.multiusers SET email = $email, - isEmailVerified = false + isEmailVerified = false, + emailChangeDate = NOW() WHERE _id = $multiUserId""".asUpdate) } yield () diff --git a/app/models/user/UserService.scala b/app/models/user/UserService.scala index 28589717945..d148dec0ec2 100755 --- a/app/models/user/UserService.scala +++ b/app/models/user/UserService.scala @@ -198,7 +198,10 @@ class UserService @Inject()(conf: WkConf, } for { oldEmail <- emailFor(user) - _ <- Fox.runIf(oldEmail != email)(multiUserDAO.updateEmail(user._multiUser, email)) + _ <- Fox.runIf(oldEmail != email)(for { + _ <- multiUserDAO.updateEmail(user._multiUser, email) + _ = logger.info(s"Email of MultiUser ${user._multiUser} changed from $oldEmail to $email") + } yield ()) _ <- userDAO.updateValues(user._id, firstName, lastName, diff --git a/conf/application.conf b/conf/application.conf index 951c4611ad8..0657f8f17a4 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -83,8 +83,8 @@ webKnossos { inviteExpiry = 14 days ssoKey = "" emailVerification { - activated = false - required = false + activated = true + required = true gracePeriod = 7 days # time period in which users do not need to verify their email address linkExpiry = 30 days } diff --git a/conf/evolutions/135-extra-column-for-email-changed.sql b/conf/evolutions/135-extra-column-for-email-changed.sql new file mode 100644 index 00000000000..ded6a5fa594 --- /dev/null +++ b/conf/evolutions/135-extra-column-for-email-changed.sql @@ -0,0 +1,24 @@ +START TRANSACTION; + +do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 134, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql; + +DROP VIEW IF EXISTS webknossos.userInfos; +DROP VIEW IF EXISTS webknossos.multiUsers_; + +ALTER TABLE webknossos.multiUsers ADD COLUMN emailChangeDate TIMESTAMPTZ NOT NULL DEFAULT NOW(); +UPDATE webknossos.multiUsers SET emailChangeDate = created; + +CREATE VIEW webknossos.multiUsers_ AS SELECT * FROM webknossos.multiUsers WHERE NOT isDeleted; +CREATE VIEW webknossos.userInfos AS +SELECT +u._id AS _user, m.email, u.firstName, u.lastname, o.name AS organization_name, +u.isDeactivated, u.isDatasetManager, u.isAdmin, m.isSuperUser, +u._organization, o._id AS organization_id, u.created AS user_created, +m.created AS multiuser_created, u._multiUser, m._lastLoggedInIdentity, u.lastActivity, m.isEmailVerified +FROM webknossos.users_ u +JOIN webknossos.organizations_ o ON u._organization = o._id +JOIN webknossos.multiUsers_ m on u._multiUser = m._id; + +UPDATE webknossos.releaseInformation SET schemaVersion = 135; + +COMMIT TRANSACTION; diff --git a/conf/evolutions/reversions/135-extra-column-for-email-changed.sql b/conf/evolutions/reversions/135-extra-column-for-email-changed.sql new file mode 100644 index 00000000000..7f9217d9e64 --- /dev/null +++ b/conf/evolutions/reversions/135-extra-column-for-email-changed.sql @@ -0,0 +1,23 @@ +START TRANSACTION; + +do $$ begin ASSERT (select schemaVersion from webknossos.releaseInformation) = 135, 'Previous schema version mismatch'; end; $$ LANGUAGE plpgsql; + +DROP VIEW IF EXISTS webknossos.userInfos; +DROP VIEW IF EXISTS webknossos.multiUsers_; + +ALTER TABLE webknossos.multiUsers DROP COLUMN emailChangeDate; + +CREATE VIEW webknossos.multiUsers_ AS SELECT * FROM webknossos.multiUsers WHERE NOT isDeleted; +CREATE VIEW webknossos.userInfos AS +SELECT +u._id AS _user, m.email, u.firstName, u.lastname, o.name AS organization_name, +u.isDeactivated, u.isDatasetManager, u.isAdmin, m.isSuperUser, +u._organization, o._id AS organization_id, u.created AS user_created, +m.created AS multiuser_created, u._multiUser, m._lastLoggedInIdentity, u.lastActivity, m.isEmailVerified +FROM webknossos.users_ u +JOIN webknossos.organizations_ o ON u._organization = o._id +JOIN webknossos.multiUsers_ m on u._multiUser = m._id; + +UPDATE webknossos.releaseInformation SET schemaVersion = 134; + +COMMIT TRANSACTION; diff --git a/conf/messages b/conf/messages index 60bbba21dbc..24e36dbcc34 100644 --- a/conf/messages +++ b/conf/messages @@ -59,6 +59,7 @@ user.email.verification.keyInvalid=Verification key is invalid. user.email.verification.keyUsed=Verification key has already been used. user.email.verification.emailDoesNotMatch=This verification key is associated with a different email address. user.email.verification.linkExpired=The email verification link is expired. +user.email.change.passwordWrong=The password you entered is incorrect. user.firstName.invalid=Please check your first name for any special characters user.lastName.invalid=Please check your last name for any special characters user.configuration.invalid=Could not parse configuration diff --git a/frontend/javascripts/admin/auth/change_email_view.tsx b/frontend/javascripts/admin/auth/change_email_view.tsx new file mode 100644 index 00000000000..0909d0c77f8 --- /dev/null +++ b/frontend/javascripts/admin/auth/change_email_view.tsx @@ -0,0 +1,176 @@ +import { LockOutlined, MailOutlined } from "@ant-design/icons"; +import { updateUser } from "admin/rest_api"; +import { Alert, Button, Col, Form, Input, Row } from "antd"; +import { useWkSelector } from "libs/react_hooks"; +import Request from "libs/request"; +import Toast from "libs/toast"; +import { type RouteComponentProps, withRouter } from "react-router-dom"; +import { logoutUserAction } from "viewer/model/actions/user_actions"; +import { Store } from "viewer/singletons"; +import { handleResendVerificationEmail } from "./verify_email_view"; + +const FormItem = Form.Item; + +const NEW_EMAIL_FIELD_KEY = "newEmail"; +const CONFIRM_NEW_EMAIL_FIELD_KEY = "confirmNewEmail"; +const PASSWORD_FIELD_KEY = "password"; + +function ChangeEmailView() { + const [form] = Form.useForm(); + const activeUser = useWkSelector((state) => state.activeUser); + + async function changeEmail(newEmail: string, password: string) { + const newUser = Object.assign({}, activeUser, { + email: newEmail, + password, + }); + return updateUser(newUser); + } + + function onFinish() { + const newEmail = form.getFieldValue(NEW_EMAIL_FIELD_KEY); + const password = form.getFieldValue(PASSWORD_FIELD_KEY); + changeEmail(newEmail, password) + .then(async () => { + handleResendVerificationEmail(); + Toast.success("Email address changed successfully. You will be logged out."); + await Request.receiveJSON("/api/auth/logout"); + window.location.href = "/auth/login"; + Store.dispatch(logoutUserAction()); + }) + .catch((error) => { + const errorMsg = "An unexpected error occurred while changing the email address."; + Toast.error(errorMsg); + console.error(errorMsg, error); + }); + } + + function checkEmailsAreMatching(value: string, otherEmailFieldKey: string[]) { + const otherFieldValue = form.getFieldValue(otherEmailFieldKey); + + if (value && otherFieldValue) { + if (value !== otherFieldValue) { + return Promise.reject(new Error("Email addresses do not match")); + } else if (form.getFieldError(otherEmailFieldKey).length > 0) { + form.validateFields([otherEmailFieldKey]); + } + } + + return Promise.resolve(); + } + + return ( + + +

Change Email

+ +
+ + + } + placeholder="Your Password" + /> + + + checkEmailsAreMatching(value, [CONFIRM_NEW_EMAIL_FIELD_KEY]), + }, + ]} + > + + } + placeholder="New Email Address" + /> + + + checkEmailsAreMatching(value, [NEW_EMAIL_FIELD_KEY]), + }, + ]} + > + + } + placeholder="Confirm New Email Address" + /> + + + + +
+ + + ); +} + +export default withRouter(ChangeEmailView); diff --git a/frontend/javascripts/admin/user/user_list_view.tsx b/frontend/javascripts/admin/user/user_list_view.tsx index e617fab0d7f..a91a2d4fd46 100644 --- a/frontend/javascripts/admin/user/user_list_view.tsx +++ b/frontend/javascripts/admin/user/user_list_view.tsx @@ -17,7 +17,7 @@ import { getEditableUsers, updateUser } from "admin/rest_api"; import { renderTeamRolesAndPermissionsForUser } from "admin/team/team_list_view"; import ExperienceModalView from "admin/user/experience_modal_view"; import PermissionsAndTeamsModalView from "admin/user/permissions_and_teams_modal_view"; -import { Alert, App, Button, Col, Input, Modal, Row, Spin, Table, Tag, Tooltip } from "antd"; +import { Alert, App, Button, Col, Input, Row, Spin, Table, Tag, Tooltip } from "antd"; import LinkButton from "components/link_button"; import dayjs from "dayjs"; import Persistence from "libs/persistence"; @@ -25,7 +25,6 @@ import Toast from "libs/toast"; import * as Utils from "libs/utils"; import { location } from "libs/window"; import _ from "lodash"; -import messages from "messages"; import React, { type Key, useEffect, useState } from "react"; import { connect } from "react-redux"; import type { RouteComponentProps } from "react-router-dom"; @@ -34,9 +33,6 @@ import type { APIOrganization, APITeamMembership, APIUser, ExperienceMap } from import { enforceActiveOrganization } from "viewer/model/accessors/organization_accessors"; import { enforceActiveUser } from "viewer/model/accessors/user_accessor"; import type { WebknossosState } from "viewer/store"; -import EditableTextLabel from "viewer/view/components/editable_text_label"; -import { logoutUserAction } from "../../viewer/model/actions/user_actions"; -import Store from "../../viewer/store"; const { Column } = Table; const { Search } = Input; @@ -122,28 +118,6 @@ function UserListView({ activeUser, activeOrganization }: Props) { activateUser(user, false); } - async function changeEmail(selectedUser: APIUser, newEmail: string) { - const newUserPromises = users.map((user) => { - if (selectedUser.id === user.id) { - const newUser = Object.assign({}, user, { - email: newEmail, - }); - return updateUser(newUser); - } - - return Promise.resolve(user); - }); - Promise.all(newUserPromises).then( - (newUsers) => { - setUsers(newUsers); - setSelectedUserIds([selectedUser.id]); - Toast.success(messages["users.change_email_confirmation"]); - if (activeUser.email === selectedUser.email) Store.dispatch(logoutUserAction()); - }, - () => {}, // Do nothing, change did not succeed - ); - } - function handleUsersChange(updatedUsers: Array): void { setUsers(updatedUsers); setIsExperienceModalOpen(false); @@ -423,33 +397,6 @@ function UserListView({ activeUser, activeOrganization }: Props) { key="email" width={320} sorter={Utils.localeCompareBy((user) => user.email)} - render={(__, user: APIUser) => - activeUser.isAdmin ? ( - { - if (newEmail !== user.email) { - Modal.confirm({ - title: messages["users.change_email_title"], - content: messages["users.change_email"]({ - newEmail, - }), - onOk: () => changeEmail(user, newEmail), - }); - } - }} - /> - ) : ( - user.email - ) - } /> Change Password, }, + { key: "changeEmail", label: Change Email }, { key: "token", label: Auth Token }, { key: "theme", diff --git a/frontend/javascripts/router.tsx b/frontend/javascripts/router.tsx index 6176b7aa097..0cd29bb55fd 100644 --- a/frontend/javascripts/router.tsx +++ b/frontend/javascripts/router.tsx @@ -1,5 +1,6 @@ import AcceptInviteView from "admin/auth/accept_invite_view"; import AuthTokenView from "admin/auth/auth_token_view"; +import ChangeEmailView from "admin/auth/change_email_view"; import ChangePasswordView from "admin/auth/change_password_view"; import FinishResetPasswordView from "admin/auth/finish_reset_password_view"; import LoginView from "admin/auth/login_view"; @@ -644,6 +645,11 @@ class ReactRouter extends React.Component { path="/auth/token" component={AuthTokenView} /> +