Improve sign-up and profile updates (#1581)

asreview/webapp/api/auth.py

@@ -220,6 +220,8 @@ def signup():
                 else:
                     # result is a 201 with message
                     result = (201, f'User "{identifier}" created.')
+            except ValueError as e:
+                result = (400, f"Unable to create your account! Reason: {str(e)}")
             except IntegrityError as e:
                 DB.session.rollback()
                 result = (403, f"Unable to create your account! Reason: {str(e)}")
@@ -386,15 +388,17 @@ def update_profile():
         email = request.form.get("email", "").strip()
         name = request.form.get("name", "").strip()
         affiliation = request.form.get("affiliation", "").strip()
-        password = request.form.get("password", None)
+        old_password = request.form.get("old_password", None)
+        new_password = request.form.get("new_password", None)
         public = bool(int(request.form.get("public", "1")))
 
         try:
-            user = user.update_profile(email, name, affiliation, password, public)
+            user = user.update_profile(email, name, affiliation,
+                                       old_password, new_password, public)
             DB.session.commit()
             result = (200, "User profile updated.")
         except ValueError as e:
-            result = (500, f"Unable to update your profile! Reason: {str(e)}")
+            result = (400, f"Unable to update your profile! Reason: {str(e)}")
         except IntegrityError as e:
             DB.session.rollback()
             result = (500, f"Unable to update your profile! Reason: {str(e)}")
@@ -459,6 +463,8 @@ def oauth_callback():
         (identifier, email, name) = oauth_handler.get_user_credentials(
             provider, code, redirect_uri
         )
+        # make sure identifier is a string
+        identifier = str(identifier)
         # try to find this user
         user = User.query.filter(User.identifier == identifier).one_or_none()
         # flag for response (I'd like to communicate if this user was created)
@@ -480,11 +486,17 @@ def oauth_callback():
                 DB.session.add(user)
                 DB.session.commit()
                 created_account = True
+            except IntegrityError:
+                DB.session.rollback()
+                message = "OAuth: integrity error, verify if you " + \
+                    "already have created an account!"
+                # return this immediately
+                return jsonify({"message": message}), 409
             except SQLAlchemyError:
                 DB.session.rollback()
                 message = "OAuth: unable to create your account!"
                 # return this immediately
-                return jsonify({"data": message}), 500
+                return jsonify({"message": message}), 409
 
         # log in the existing/created user immediately
         logged_in = perform_login_user(user)
@@ -498,7 +510,7 @@ def oauth_callback():
             },
         )
     else:
-        result = (400, {"data": f"OAuth provider {provider} could not be found"})
+        result = (400, {"message": f"OAuth provider {provider} could not be found"})
 
     status, message = result
     response = jsonify(message)

asreview/webapp/authentication/models.py

@@ -34,7 +34,7 @@
 from asreview.webapp import DB
 
 PASSWORD_REGEX = (
-    r"^(?=.*[A-Za-z])(?=.*\d)(?=.*[@$!%*?&#])[A-Za-z\d@$!%*?&#]{8,}$"  # noqa
+    r"^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).{8,}$"  # noqa
 )
 EMAIL_REGEX = r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,7}\b"
 
@@ -117,12 +117,24 @@ def __init__(
         self.confirmed = confirmed
         self.public = public
 
-    def update_profile(self, email, name, affiliation, password=None, public=True):
+    def update_profile(self, email, name, affiliation,
+                       old_password=None, new_password=None, public=True):
+
+        # if there is a request to update the password, and the origin
+        # is correct
+        if bool(old_password) and bool(new_password) and \
+                self.origin == "asreview":
+            # verify the old password
+            if not self.verify_password(old_password):
+                raise ValueError("Provided old password is incorrect.")
+            else:
+                # old password is verified. The following line will raise
+                # a ValueError if the new password is wrong
+                self.hashed_password = User.create_password_hash(new_password)
+
         self.email = email
         self.name = name
         self.affiliation = affiliation
-        if self.origin == "asreview" and password is not None:
-            self.hashed_password = User.create_password_hash(password)
         self.public = public
 
         return self
@@ -201,7 +213,7 @@ def create_password_hash(cls, password):
         if bool(password) and User.valid_password(password):
             return generate_password_hash(password)
         else:
-            raise ValueError(f'Password "{password}" does not meet requirements')
+            raise ValueError(f'Password "{password}" does not meet requirements.')
 
     def __repr__(self):
         return f"<User {self.email!r}, id: {self.id}>"

asreview/webapp/src/Components/SignInForm.js

@@ -101,6 +101,8 @@ const SignInForm = (props) => {
       <Stack spacing={3}>
         <TextField
           label="Email"
+          name="email"
+          type="email"
           value={email}
           onChange={handleEmailChange}
           variant="outlined"

asreview/webapp/src/Components/SignInOAuth.js

@@ -64,9 +64,7 @@ const SignInOauth = (props) => {
         }
       })
       .catch((err) => {
-        message = "Did not receive OAuth data from backend";
-        console.error(message, err);
-        setErrorMessage(message);
+        setErrorMessage(err.message);
       });
   };
 

asreview/webapp/src/Components/SignUpForm.js

@@ -18,7 +18,7 @@ import {
 } from "@mui/material";
 
 import { InlineErrorHandler } from ".";
-import { WordmarkState } from "../globals";
+import { WordmarkState, passwordValidation } from "../globals";
 import { styled } from "@mui/material/styles";
 import { HelpPrivacyTermsButton } from "../Components";
 import { useToggle } from "../hooks/useToggle";
@@ -70,12 +70,7 @@ const SignupSchema = Yup.object().shape({
   affiliation: Yup.string()
     .min(2, "Affiliation must be at least 2 characters long")
     .required("Affiliation is required"),
-  password: Yup.string()
-    .matches(
-      /^(?=.*[A-Za-z])(?=.*\d)(?=.*[@$!%*?&#])[A-Za-z\d@$!%*?&#]{8,}$/,
-      "Use 8 or more characters with a mix of letters, numbers & symbols"
-    )
-    .required("Password is required"),
+  password: passwordValidation(Yup.string()).required("Password is required"),
   confirmPassword: Yup.string()
     .required("Password confirmation is required")
     .oneOf([Yup.ref("password"), null], "Passwords must match"),
@@ -155,8 +150,10 @@ const SignUpForm = (props) => {
                 >
                   <TextField
                     id="email"
+                    name="email"
                     label="Email"
                     size="small"
+                    type="email"
                     fullWidth
                     value={formik.values.email}
                     onChange={formik.handleChange}
@@ -167,6 +164,7 @@ const SignUpForm = (props) => {
                   ) : null}
                   <TextField
                     id="name"
+                    name="name"
                     label="Full name"
                     size="small"
                     fullWidth
@@ -181,6 +179,7 @@ const SignUpForm = (props) => {
                     id="affiliation"
                     label="Affiliation"
                     size="small"
+                    name="affiliation"
                     fullWidth
                     value={formik.values.affiliation}
                     onChange={formik.handleChange}
@@ -195,6 +194,7 @@ const SignUpForm = (props) => {
                         id="password"
                         label="Password"
                         size="small"
+                        autoComplete="new-password"
                         fullWidth
                         type={returnType()}
                         value={formik.values.password}

asreview/webapp/src/HomeComponents/DashboardComponents/ProfilePage.js

@@ -5,6 +5,7 @@ import DashboardPage from "./DashboardPage";
 import {
   Box,
   Checkbox,
+  Divider,
   FormControl,
   FormControlLabel,
   FormHelperText as FHT,
@@ -14,32 +15,33 @@ import {
 } from "@mui/material";
 
 import LoadingButton from "@mui/lab/LoadingButton";
-import {
-  TypographyH5Medium,
-  TypographyH6Medium,
-} from "../../StyledComponents/StyledTypography.js";
+import { TypographyH5Medium } from "../../StyledComponents/StyledTypography.js";
 import { InlineErrorHandler } from "../../Components";
 import { useToggle } from "../../hooks/useToggle";
 
 import { AuthAPI } from "../../api";
+import { passwordValidation } from "../../globals";
 import { useFormik } from "formik";
 import * as Yup from "yup";
 
 // VALIDATION SCHEMA
 const SignupSchema = Yup.object().shape({
-  email: Yup.string().email("Invalid email").required("Email is required"),
-  name: Yup.string().required("Full name is required"),
+  email: Yup.string()
+    .email("Invalid email")
+    .required("Email is required")
+    .nullable(),
+  name: Yup.string().required("Full name is required").nullable(),
   affiliation: Yup.string()
     .min(2, "Affiliation must be at least 2 characters long")
     .nullable(),
-  password: Yup.string().matches(
-    /^(?=.*[A-Za-z])(?=.*\d)(?=.*[@$!%*?&#])[A-Za-z\d@$!%*?&#]{8,}$/,
-    "Use 8 or more characters with a mix of letters, numbers & symbols",
-  ),
-  confirmPassword: Yup.string().oneOf(
-    [Yup.ref("password"), null],
-    "Passwords must match",
-  ),
+  oldPassword: Yup.string(),
+  newPassword: passwordValidation(Yup.string()),
+  confirmPassword: Yup.string()
+    .oneOf([Yup.ref("newPassword")], "Passwords must match")
+    .when("newPassword", {
+      is: (value) => value !== undefined && value.length > 0,
+      then: (schema) => schema.required("Confirmation password is required"),
+    }),
 });
 
 const ProfilePage = (props) => {
@@ -55,6 +57,9 @@ const ProfilePage = (props) => {
     onSuccess: () => {
       navigate("/projects");
     },
+    onError: (err) => {
+      console.log(err);
+    },
   });
 
   const handleSubmit = () => {
@@ -67,7 +72,8 @@ const ProfilePage = (props) => {
     email: "",
     name: "",
     affiliation: "",
-    password: "",
+    oldPassword: "",
+    newPassword: "",
     confirmPassword: "",
     publicAccount: true,
   };
@@ -84,7 +90,7 @@ const ProfilePage = (props) => {
       formik.setFieldValue(
         "affiliation",
         data.message.affiliation || "",
-        false,
+        false
       );
       formik.setFieldValue("public", data.message.public || true);
       // show password field?
@@ -105,35 +111,67 @@ const ProfilePage = (props) => {
     return !showPassword ? "password" : "text";
   };
 
+  const oldPasswordHasValue = () => {
+    return (
+      formik.values.oldPassword === undefined ||
+      formik.values.oldPassword === ""
+    );
+  };
+
+  const passwordFieldOpacity = () => {
+    if (oldPasswordHasValue()) {
+      return 0.3;
+    } else {
+      return 1;
+    }
+  };
+
   const renderPasswordFields = (formik) => {
     return (
       <>
+        <Divider />
         <FormControl>
-          <Stack direction="row" spacing={2}>
+          <Stack direction="column" spacing={2}>
+            <Typography variant="h6">Change Password</Typography>
+            <TextField
+              id="oldPassword"
+              label="Old Password"
+              size="small"
+              fullWidth
+              type={returnType()}
+              value={formik.values.oldPassword}
+              onChange={formik.handleChange}
+              onBlur={formik.handleBlur}
+              autoComplete="off"
+            />
             <TextField
-              id="password"
-              label="Change Password"
+              id="newPassword"
+              label="New Password"
               size="small"
               fullWidth
               type={returnType()}
-              value={formik.values.password}
+              value={formik.values.newPassword}
               onChange={formik.handleChange}
               onBlur={formik.handleBlur}
+              style={{ opacity: passwordFieldOpacity() }}
+              disabled={oldPasswordHasValue()}
             />
             <TextField
               id="confirmPassword"
-              label="Confirm Password"
+              label="Confirm New Password"
               size="small"
               fullWidth
               type={returnType()}
               value={formik.values.confirmPassword}
               onChange={formik.handleChange}
               onBlur={formik.handleBlur}
+              style={{ opacity: passwordFieldOpacity() }}
+              disabled={oldPasswordHasValue()}
             />
           </Stack>
         </FormControl>
-        {formik.touched.password && formik.errors.password ? (
-          <FHT error={true}>{formik.errors.password}</FHT>
+        {formik.touched.newPassword && formik.errors.newPassword ? (
+          <FHT error={true}>{formik.errors.newPassword}</FHT>
         ) : null}
         {formik.touched.confirmPassword && formik.errors.confirmPassword ? (
           <FHT error={true}>{formik.errors.confirmPassword}</FHT>
@@ -147,7 +185,7 @@ const ProfilePage = (props) => {
                 onChange={toggleShowPassword}
               />
             }
-            label="Show password"
+            label="Show passwords"
           />
         </FormControl>
       </>
@@ -173,7 +211,8 @@ const ProfilePage = (props) => {
               <Stack direction="row" spacing={1}>
                 <span>
                   <LoadingButton
-                    /*disabled={!formik.isValid}*/
+                    id="save"
+                    disabled={!formik.isValid}
                     loading={loadingSaveButton}
                     variant="contained"
                     onClick={handleSubmit}
@@ -190,9 +229,12 @@ const ProfilePage = (props) => {
           <Box className="main-page-body-wrapper">
             <Stack className="main-page-body" direction={"column"} spacing={3}>
               {showFirstTimeMessage && (
-                <TypographyH6Medium>
+                <Typography variant="h6">
                   Please take a second to review your profile data:
-                </TypographyH6Medium>
+                </Typography>
+              )}
+              {!showFirstTimeMessage && (
+                <Typography variant="h6">User data</Typography>
               )}
               <TextField
                 autoFocus
@@ -232,6 +274,7 @@ const ProfilePage = (props) => {
                 <FHT error={true}>{formik.errors.affiliation}</FHT>
               ) : null}
               {showPasswordFields && renderPasswordFields(formik)}
+
               {false && (
                 <>
                   <FormControlLabel