Require that all passwords are between 8 and 64 characters (#1239)

- Require that all passwords are between 8 and 64 characters
- Fixes account settings password reset form to only trigger
logged-in event after successful password change.
- Password validation can be extended within the UserManager's
validate_password method to add or modify requirements.
- Add tests for password validation

backend/btrixcloud/users.py

@@ -6,7 +6,7 @@ import os
 import uuid
 import asyncio
 
-from typing import Optional
+from typing import Optional, Union
 
 from pydantic import UUID4
 import passlib.pwd
@@ -17,7 +17,7 @@ from fastapi.security import OAuth2PasswordBearer
 from pymongo.errors import DuplicateKeyError
 
 from fastapi_users import FastAPIUsers, BaseUserManager
-from fastapi_users.manager import UserAlreadyExists
+from fastapi_users.manager import UserAlreadyExists, InvalidPasswordException
 from fastapi_users.authentication import (
     AuthenticationBackend,
     BearerTransport,
@@ -96,6 +96,23 @@ class UserManager(BaseUserManager[UserCreate, UserDB]):
         await self.on_after_register_custom(created_user, user, request)
         return created_user
 
+    async def validate_password(
+        self, password: str, user: Union[UserCreate, UserDB]
+    ) -> None:
+        """
+        Validate a password.
+
+        Overloaded to set password requirements.
+
+        :param password: The password to validate.
+        :param user: The user associated to this password.
+        :raises InvalidPasswordException: The password is invalid.
+        :return: None if the password is valid.
+        """
+        pw_length = len(password)
+        if not 8 <= pw_length <= 64:
+            raise InvalidPasswordException(reason="invalid_password_length")
+
     async def get_user_names_by_ids(self, user_ids):
         """return list of user names for given ids"""
         user_ids = [UUID4(id_) for id_ in user_ids]
@@ -147,9 +164,11 @@ class UserManager(BaseUserManager[UserCreate, UserDB]):
             )
             print(f"Super user {email} created", flush=True)
             print(res, flush=True)
-
         except (DuplicateKeyError, UserAlreadyExists):
             print(f"User {email} already exists", flush=True)
+        # pylint: disable=raise-missing-from
+        except InvalidPasswordException:
+            raise HTTPException(status_code=422, detail="invalid_password")
 
     async def create_non_super_user(
         self,
@@ -174,9 +193,17 @@ class UserManager(BaseUserManager[UserCreate, UserDB]):
                 newOrg=False,
                 is_verified=True,
             )
-            created_user = await super().create(user_create, safe=False, request=None)
+            try:
-            await self.on_after_register_custom(created_user, user_create, request=None)
+                created_user = await super().create(
-            return created_user
+                    user_create, safe=False, request=None
+                )
+                await self.on_after_register_custom(
+                    created_user, user_create, request=None
+                )
+                return created_user
+            # pylint: disable=raise-missing-from
+            except InvalidPasswordException:
+                raise HTTPException(status_code=422, detail="invalid_password")
 
         except (DuplicateKeyError, UserAlreadyExists):
             print(f"User {email} already exists", flush=True)

backend/test/test_org.py

@@ -226,7 +226,7 @@ def test_send_and_accept_org_invite(
         json={
             "name": "accepted",
             "email": expected_stored_email,
-            "password": "testpw",
+            "password": "testingpassword",
             "inviteToken": token,
             "newOrg": False,
         },

backend/test/test_users.py

@@ -1,6 +1,10 @@
 import requests
+import time
 
-from .conftest import API_PREFIX, CRAWLER_USERNAME
+from .conftest import API_PREFIX, CRAWLER_USERNAME, ADMIN_PW, ADMIN_USERNAME
+
+VALID_USER_EMAIL = "validpassword@example.com"
+VALID_USER_PW = "validpassw0rd!"
 
 
 def test_create_super_user(admin_auth_headers):
@@ -42,3 +46,139 @@ def test_me_with_orgs(crawler_auth_headers, default_org_id):
     assert default_org["name"]
     assert default_org["default"]
     assert default_org["role"] == 20
+
+
+def test_add_user_to_org_invalid_password(admin_auth_headers, default_org_id):
+    r = requests.post(
+        f"{API_PREFIX}/orgs/{default_org_id}/add-user",
+        json={
+            "email": "invalidpassword@example.com",
+            "password": "pw",
+            "name": "invalid pw user",
+            "description": "test invalid password",
+            "role": 20,
+        },
+        headers=admin_auth_headers,
+    )
+    assert r.status_code == 422
+    assert r.json()["detail"] == "invalid_password"
+
+
+def test_register_user_invalid_password(admin_auth_headers, default_org_id):
+    email = "invalidpassword@example.com"
+    # Send invite
+    r = requests.post(
+        f"{API_PREFIX}/orgs/{default_org_id}/invite",
+        headers=admin_auth_headers,
+        json={"email": email, "role": 20},
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["invited"] == "new_user"
+
+    # Look up token
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/invites",
+        headers=admin_auth_headers,
+    )
+    assert r.status_code == 200
+    data = r.json()
+    invites_matching_email = [
+        invite for invite in data["items"] if invite["email"] == email
+    ]
+    token = invites_matching_email[0]["id"]
+
+    # Create user with invite
+    r = requests.post(
+        f"{API_PREFIX}/auth/register",
+        headers=admin_auth_headers,
+        json={
+            "name": "invalid",
+            "email": email,
+            "password": "passwd",
+            "inviteToken": token,
+            "newOrg": False,
+        },
+    )
+    assert r.status_code == 400
+    detail = r.json()["detail"]
+    assert detail["code"] == "REGISTER_INVALID_PASSWORD"
+    assert detail["reason"] == "invalid_password_length"
+
+
+def test_register_user_valid_password(admin_auth_headers, default_org_id):
+    # Send invite
+    r = requests.post(
+        f"{API_PREFIX}/orgs/{default_org_id}/invite",
+        headers=admin_auth_headers,
+        json={"email": VALID_USER_EMAIL, "role": 20},
+    )
+    assert r.status_code == 200
+    data = r.json()
+    assert data["invited"] == "new_user"
+
+    # Look up token
+    r = requests.get(
+        f"{API_PREFIX}/orgs/{default_org_id}/invites",
+        headers=admin_auth_headers,
+    )
+    assert r.status_code == 200
+    data = r.json()
+    invites_matching_email = [
+        invite for invite in data["items"] if invite["email"] == VALID_USER_EMAIL
+    ]
+    token = invites_matching_email[0]["id"]
+
+    # Create user with invite
+    r = requests.post(
+        f"{API_PREFIX}/auth/register",
+        headers=admin_auth_headers,
+        json={
+            "name": "valid",
+            "email": VALID_USER_EMAIL,
+            "password": VALID_USER_PW,
+            "inviteToken": token,
+            "newOrg": False,
+        },
+    )
+    assert r.status_code == 201
+
+
+def test_reset_invalid_password(admin_auth_headers):
+    r = requests.patch(
+        f"{API_PREFIX}/users/me",
+        headers=admin_auth_headers,
+        json={"email": ADMIN_USERNAME, "password": "12345"},
+    )
+    assert r.status_code == 400
+    detail = r.json()["detail"]
+    assert detail["code"] == "UPDATE_USER_INVALID_PASSWORD"
+    assert detail["reason"] == "invalid_password_length"
+
+
+def test_reset_valid_password(admin_auth_headers, default_org_id):
+    valid_user_headers = {}
+    while True:
+        r = requests.post(
+            f"{API_PREFIX}/auth/jwt/login",
+            data={
+                "username": VALID_USER_EMAIL,
+                "password": VALID_USER_PW,
+                "grant_type": "password",
+            },
+        )
+        data = r.json()
+        try:
+            valid_user_headers = {"Authorization": f"Bearer {data['access_token']}"}
+            break
+        except:
+            print("Waiting for valid user auth headers")
+            time.sleep(5)
+
+    r = requests.patch(
+        f"{API_PREFIX}/users/me",
+        headers=valid_user_headers,
+        json={"email": VALID_USER_EMAIL, "password": "new!password"},
+    )
+    assert r.status_code == 200
+    assert r.json()["email"] == VALID_USER_EMAIL

frontend/src/components/account-settings.ts

@@ -311,6 +311,8 @@ export class AccountSettings extends LiteElement {
             name="newPassword"
             type="password"
             label="${msg("New password")}"
+            help-text=${msg("Must be between 8-64 characters")}
+            minlength="8"
             autocomplete="new-password"
             password-toggle
             required
@@ -351,8 +353,6 @@ export class AccountSettings extends LiteElement {
         email: this.authState.username,
         password: formData.get("password") as string,
       });
-
-      this.dispatchEvent(AuthService.createLoggedInEvent(nextAuthState));
     } catch (e: any) {
       console.debug(e);
     }
@@ -371,6 +371,7 @@ export class AccountSettings extends LiteElement {
 
     const params = {
       password: formData.get("newPassword"),
+      email: this.userInfo?.email,
     };
 
     try {
@@ -385,13 +386,17 @@ export class AccountSettings extends LiteElement {
           successMessage: "Successfully updated password",
         },
       });
+
+      this.dispatchEvent(AuthService.createLoggedInEvent(nextAuthState));
     } catch (e) {
       console.error(e);
 
       this.formStateService.send({
         type: "ERROR",
         detail: {
-          serverError: msg("Something went wrong changing password"),
+          serverError: msg(
+            "Something went wrong changing password. Verify that new password meets requirements (8-64 characters)."
+          ),
         },
       });
     }

frontend/src/components/sign-up-form.ts

@@ -79,7 +79,9 @@ export class SignUpForm extends LiteElement {
             id="password"
             name="password"
             type="password"
-            label=${msg("Create a password")}
+            label="${msg("New password")}"
+            help-text=${msg("Must be between 8-64 characters")}
+            minlength="8"
             autocomplete="new-password"
             passwordToggle
             required
@@ -172,9 +174,12 @@ export class SignUpForm extends LiteElement {
         const { detail } = await resp.json();
         if (detail === "REGISTER_USER_ALREADY_EXISTS") {
           shouldLogIn = true;
+        } else if (detail.code && detail.code === "REGISTER_INVALID_PASSWORD") {
+          this.serverError = msg(
+            "Invalid password. Must be between 8 and 64 characters"
+          );
         } else {
-          // TODO show validation details
+          this.serverError = msg("Invalid email or password");
-          this.serverError = msg("Invalid email address or password");
         }
         break;
       default:

frontend/src/pages/reset-password.ts

@@ -38,6 +38,8 @@ export class ResetPassword extends LiteElement {
                 name="password"
                 type="password"
                 label="${msg("New password")}"
+                help-text=${msg("Must be between 8-64 characters")}
+                minlength="8"
                 autocomplete="new-password"
                 passwordToggle
                 required
@@ -95,17 +97,19 @@ export class ResetPassword extends LiteElement {
       case 400:
       case 422:
         const { detail } = await resp.json();
-
         if (detail === "RESET_PASSWORD_BAD_TOKEN") {
           // TODO password validation details
           this.serverError = msg(
             "Password reset email is not valid. Request a new password reset email"
           );
-        } else {
+        } else if (
-          // TODO password validation details
+          detail.code &&
-          this.serverError = msg("Invalid password");
+          detail.code === "RESET_PASSWORD_INVALID_PASSWORD"
+        ) {
+          this.serverError = msg(
+            "Invalid password. Must be between 8 and 64 characters"
+          );
         }
-
         break;
       default:
         this.serverError = msg("Something unexpected went wrong");