From a31d09f49432f907195da160ec2f71c6d0093819 Mon Sep 17 00:00:00 2001 From: Shahar Glazner Date: Mon, 29 Jun 2026 15:35:43 +0300 Subject: [PATCH] feat(auth): allow local account users to change their password Local (DB) accounts previously had no way to change a password after creation. This adds: - Self-service `PUT /auth/users/me/password` endpoint that verifies the current password before updating (works for any authenticated local user, admin or noc). - `DbIdentityManager.update_user` now supports password/role updates so admins can reset a local user's password via `PUT /auth/users/{user}`. - `update_user_password` DB helper. - "Change Password" item in the user dropdown (DB auth only) with a modal. - Optional password reset field for existing users in the admin sidebar. Closes #6608 Co-authored-by: Cursor --- .../(keep)/settings/auth/users-sidebar.tsx | 16 +- .../components/navbar/ChangePasswordModal.tsx | 144 ++++++++++++++ keep-ui/components/navbar/UserInfo.tsx | 22 +++ .../__tests__/ChangePasswordModal.test.tsx | 108 +++++++++++ keep/api/core/db.py | 19 ++ keep/api/routes/auth/users.py | 50 +++++ .../db/db_identitymanager.py | 32 +++- tests/test_change_password.py | 179 ++++++++++++++++++ 8 files changed, 566 insertions(+), 4 deletions(-) create mode 100644 keep-ui/components/navbar/ChangePasswordModal.tsx create mode 100644 keep-ui/components/navbar/__tests__/ChangePasswordModal.test.tsx create mode 100644 tests/test_change_password.py diff --git a/keep-ui/app/(keep)/settings/auth/users-sidebar.tsx b/keep-ui/app/(keep)/settings/auth/users-sidebar.tsx index a5d336a7c3..2e5bdb9dde 100644 --- a/keep-ui/app/(keep)/settings/auth/users-sidebar.tsx +++ b/keep-ui/app/(keep)/settings/auth/users-sidebar.tsx @@ -266,17 +266,27 @@ const UsersSidebar = ({ )} {/* Password Field */} {(authType === AuthType.DB || authType === AuthType.KEYCLOAK) && - isNewUser && userCreationAllowed && (
- Password + + {isNewUser ? "Password" : "Reset Password"} + ( void; +} + +export const ChangePasswordModal = ({ + isOpen, + onClose, +}: ChangePasswordModalProps) => { + const api = useApi(); + const [currentPassword, setCurrentPassword] = useState(""); + const [newPassword, setNewPassword] = useState(""); + const [confirmPassword, setConfirmPassword] = useState(""); + const [error, setError] = useState(null); + const [isSubmitting, setIsSubmitting] = useState(false); + + const resetForm = () => { + setCurrentPassword(""); + setNewPassword(""); + setConfirmPassword(""); + setError(null); + setIsSubmitting(false); + }; + + const handleClose = () => { + resetForm(); + onClose(); + }; + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + setError(null); + + if (!currentPassword) { + setError("Current password is required"); + return; + } + if (!newPassword) { + setError("New password is required"); + return; + } + if (newPassword !== confirmPassword) { + setError("New password and confirmation do not match"); + return; + } + if (newPassword === currentPassword) { + setError("New password must be different from the current password"); + return; + } + + setIsSubmitting(true); + try { + await api.put("/auth/users/me/password", { + current_password: currentPassword, + new_password: newPassword, + }); + showSuccessToast("Password changed successfully"); + handleClose(); + } catch (err) { + if (err instanceof KeepApiError) { + setError(err.message || "Failed to change password"); + } else { + setError("An unexpected error occurred"); + } + } finally { + setIsSubmitting(false); + } + }; + + return ( + +
+
+ Current Password + setCurrentPassword(e.target.value)} + autoComplete="current-password" + /> +
+
+ New Password + setNewPassword(e.target.value)} + autoComplete="new-password" + /> +
+
+ Confirm New Password + setConfirmPassword(e.target.value)} + autoComplete="new-password" + /> +
+ {error && ( + + {error} + + )} +
+ + +
+
+ + ); +}; diff --git a/keep-ui/components/navbar/UserInfo.tsx b/keep-ui/components/navbar/UserInfo.tsx index 093ddfd523..10b604fe74 100644 --- a/keep-ui/components/navbar/UserInfo.tsx +++ b/keep-ui/components/navbar/UserInfo.tsx @@ -1,5 +1,6 @@ "use client"; +import { useState } from "react"; import { Menu } from "@headlessui/react"; import { LinkWithIcon } from "components/LinkWithIcon"; import { Session } from "next-auth"; @@ -14,6 +15,7 @@ import { useSignOut } from "@/shared/lib/hooks/useSignOut"; import { FaSlack } from "react-icons/fa"; import { ThemeControl } from "@/shared/ui"; import { HiOutlineDocumentText } from "react-icons/hi2"; +import { ChangePasswordModal } from "./ChangePasswordModal"; const ONBOARDING_FLOW_ID = "flow_FHDz1hit"; @@ -24,6 +26,7 @@ type UserDropdownProps = { const UserDropdown = ({ session }: UserDropdownProps) => { const { data: configData } = useConfig(); const signOut = useSignOut(); + const [isChangePasswordOpen, setIsChangePasswordOpen] = useState(false); const { refs, floatingStyles } = useFloating({ placement: "right-end", strategy: "fixed", @@ -36,6 +39,8 @@ const UserDropdown = ({ session }: UserDropdownProps) => { const { name, image, email } = user; const isNoAuth = configData?.AUTH_TYPE === AuthType.NOAUTH; + // Self-service password change is only supported for local (DB) accounts. + const canChangePassword = configData?.AUTH_TYPE === AuthType.DB; return ( @@ -65,6 +70,17 @@ const UserDropdown = ({ session }: UserDropdownProps) => { )} + {canChangePassword && ( +
  • + setIsChangePasswordOpen(true)} + > + Change Password + +
    • + )} {!isNoAuth && (
  • { )}
    • + {canChangePassword && ( + setIsChangePasswordOpen(false)} + /> + )} ); }; diff --git a/keep-ui/components/navbar/__tests__/ChangePasswordModal.test.tsx b/keep-ui/components/navbar/__tests__/ChangePasswordModal.test.tsx new file mode 100644 index 0000000000..2998e51058 --- /dev/null +++ b/keep-ui/components/navbar/__tests__/ChangePasswordModal.test.tsx @@ -0,0 +1,108 @@ +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { ChangePasswordModal } from "../ChangePasswordModal"; +import { useApi } from "@/shared/lib/hooks/useApi"; +import { showSuccessToast } from "@/shared/ui"; +import { KeepApiError } from "@/shared/api"; + +jest.mock("@/shared/lib/hooks/useApi"); +jest.mock("@/shared/ui", () => ({ + showSuccessToast: jest.fn(), +})); + +describe("ChangePasswordModal", () => { + const mockPut = jest.fn(); + const mockOnClose = jest.fn(); + + beforeEach(() => { + (useApi as jest.Mock).mockReturnValue({ put: mockPut }); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + const fillForm = (current: string, next: string, confirm: string) => { + fireEvent.change( + screen.getByPlaceholderText("Enter your current password"), + { target: { value: current } } + ); + fireEvent.change(screen.getByPlaceholderText("Enter a new password"), { + target: { value: next }, + }); + fireEvent.change(screen.getByPlaceholderText("Re-enter the new password"), { + target: { value: confirm }, + }); + }; + + it("submits the password change and closes on success", async () => { + mockPut.mockResolvedValue({ status: "OK" }); + render(); + + fillForm("oldpass", "newpass", "newpass"); + fireEvent.click(screen.getByRole("button", { name: /change password/i })); + + await waitFor(() => { + expect(mockPut).toHaveBeenCalledWith("/auth/users/me/password", { + current_password: "oldpass", + new_password: "newpass", + }); + }); + expect(showSuccessToast).toHaveBeenCalledWith( + "Password changed successfully" + ); + expect(mockOnClose).toHaveBeenCalled(); + }); + + it("shows an error when passwords do not match", async () => { + render(); + + fillForm("oldpass", "newpass", "different"); + fireEvent.click(screen.getByRole("button", { name: /change password/i })); + + await waitFor(() => { + expect( + screen.getByText("New password and confirmation do not match") + ).toBeInTheDocument(); + }); + expect(mockPut).not.toHaveBeenCalled(); + }); + + it("shows an error when the new password equals the current one", async () => { + render(); + + fillForm("samepass", "samepass", "samepass"); + fireEvent.click(screen.getByRole("button", { name: /change password/i })); + + await waitFor(() => { + expect( + screen.getByText( + "New password must be different from the current password" + ) + ).toBeInTheDocument(); + }); + expect(mockPut).not.toHaveBeenCalled(); + }); + + it("surfaces the API error message on failure", async () => { + mockPut.mockRejectedValue( + new KeepApiError( + "Current password is incorrect", + "/auth/users/me/password", + "Current password is incorrect", + undefined, + 403 + ) + ); + render(); + + fillForm("wrongpass", "newpass", "newpass"); + fireEvent.click(screen.getByRole("button", { name: /change password/i })); + + await waitFor(() => { + expect( + screen.getByText("Current password is incorrect") + ).toBeInTheDocument(); + }); + expect(mockOnClose).not.toHaveBeenCalled(); + }); +}); diff --git a/keep/api/core/db.py b/keep/api/core/db.py index 140c3a6a3e..878d0d877b 100644 --- a/keep/api/core/db.py +++ b/keep/api/core/db.py @@ -2146,6 +2146,25 @@ def update_user_role(tenant_id, username, role): return user +def update_user_password(tenant_id, username, password): + from keep.api.models.db.user import User + + password_hash = hashlib.sha256(password.encode()).hexdigest() + with Session(engine) as session: + user = session.exec( + select(User) + .where(User.tenant_id == tenant_id) + .where(User.username == username) + ).first() + if not user: + return None + user.password_hash = password_hash + session.add(user) + session.commit() + session.refresh(user) + return user + + def save_workflow_results(tenant_id, workflow_execution_id, workflow_results): with Session(engine) as session: workflow_execution = session.exec( diff --git a/keep/api/routes/auth/users.py b/keep/api/routes/auth/users.py index 15d577d081..c3d56714d9 100644 --- a/keep/api/routes/auth/users.py +++ b/keep/api/routes/auth/users.py @@ -4,6 +4,9 @@ from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel, Field, validator +from keep.api.core.config import config +from keep.api.core.db import get_user as get_db_user +from keep.api.core.db import update_user_password from keep.api.models.user import User from keep.identitymanager.authenticatedentity import AuthenticatedEntity from keep.identitymanager.identitymanagerfactory import IdentityManagerFactory @@ -41,6 +44,17 @@ def validate_role(cls, v): return v +class ChangePasswordRequest(BaseModel): + current_password: str + new_password: str + + @validator("new_password", allow_reuse=True) + def validate_new_password(cls, v): + if not v or not v.strip(): + raise ValueError("New password cannot be empty") + return v + + @router.get("", description="Get all users") def get_users( authenticated_entity: AuthenticatedEntity = Depends( @@ -93,6 +107,42 @@ async def create_user( ) +@router.put( + "/me/password", + description="Change the password of the currently authenticated user", +) +def change_my_password( + request_data: ChangePasswordRequest, + authenticated_entity: AuthenticatedEntity = Depends( + IdentityManagerFactory.get_auth_verifier(["read:settings"]) + ), +): + # Self-service password change is only supported for local (DB) accounts. + auth_type = config("AUTH_TYPE", default="").lower() + if auth_type not in ("db", "single_tenant"): + raise HTTPException( + status_code=501, + detail="Changing your password is not supported for this authentication type", + ) + + username = authenticated_entity.email + if not username: + raise HTTPException(status_code=400, detail="Unable to resolve current user") + + # Verify the current password before allowing a change. + user = get_db_user(username, request_data.current_password, update_sign_in=False) + if not user: + raise HTTPException(status_code=403, detail="Current password is incorrect") + + updated_user = update_user_password( + authenticated_entity.tenant_id, username, request_data.new_password + ) + if not updated_user: + raise HTTPException(status_code=404, detail="User not found") + + return {"status": "OK"} + + @router.put("/{user_email}", description="Update a user") async def update_user( user_email: str, diff --git a/keep/identitymanager/identity_managers/db/db_identitymanager.py b/keep/identitymanager/identity_managers/db/db_identitymanager.py index b2673f1d6a..0492c77f50 100644 --- a/keep/identitymanager/identity_managers/db/db_identitymanager.py +++ b/keep/identitymanager/identity_managers/db/db_identitymanager.py @@ -8,6 +8,8 @@ from keep.api.core.db import delete_user as delete_user_from_db from keep.api.core.db import get_user from keep.api.core.db import get_users as get_users_from_db +from keep.api.core.db import update_user_password as update_user_password_in_db +from keep.api.core.db import update_user_role as update_user_role_in_db from keep.api.core.dependencies import SINGLE_TENANT_UUID from keep.api.models.user import User from keep.contextmanager.contextmanager import ContextManager @@ -110,4 +112,32 @@ def get_auth_verifier(self, scopes) -> DbAuthVerifier: return DbAuthVerifier(scopes) def update_user(self, user_email: str, update_data: dict) -> User: - raise NotImplementedError("DbIdentityManager.update_user") + # For DB auth the identifier is the username (stored as `email` in the DTO) + password = update_data.get("password") + role = update_data.get("role") + + updated_user = None + if password: + updated_user = update_user_password_in_db( + self.tenant_id, user_email, password + ) + if not updated_user: + raise HTTPException(status_code=404, detail="User not found") + + if role: + updated_user = update_user_role_in_db(self.tenant_id, user_email, role) + if not updated_user: + raise HTTPException(status_code=404, detail="User not found") + + if updated_user is None: + raise HTTPException(status_code=400, detail="No update data provided") + + return User( + email=updated_user.username, + name=updated_user.username, + role=updated_user.role, + last_login=( + str(updated_user.last_sign_in) if updated_user.last_sign_in else None + ), + created_at=str(updated_user.created_at), + ) diff --git a/tests/test_change_password.py b/tests/test_change_password.py new file mode 100644 index 0000000000..88665dce88 --- /dev/null +++ b/tests/test_change_password.py @@ -0,0 +1,179 @@ +import hashlib + +import pytest + +from keep.api.core.dependencies import SINGLE_TENANT_UUID +from tests.fixtures.client import client, test_app # noqa + + +def _create_db_user(db_session, username, password, role="admin"): + from keep.api.models.db.user import User + + db_session.add( + User( + tenant_id=SINGLE_TENANT_UUID, + username=username, + password_hash=hashlib.sha256(password.encode()).hexdigest(), + role=role, + ) + ) + db_session.commit() + + +def _signin(client, username, password): + response = client.post( + "/signin", + json={"username": username, "password": password}, + ) + return response + + +def _get_password_hash(db_session, username): + from keep.api.models.db.user import User + from sqlmodel import select + + user = db_session.exec( + select(User) + .where(User.tenant_id == SINGLE_TENANT_UUID) + .where(User.username == username) + ).first() + return user.password_hash if user else None + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_change_own_password_success(db_session, client, test_app): + """A local (DB) user can change their own password.""" + _create_db_user(db_session, "alice", "oldpassword") + + signin = _signin(client, "alice", "oldpassword") + assert signin.status_code == 200 + token = signin.json()["accessToken"] + + response = client.put( + "/auth/users/me/password", + json={"current_password": "oldpassword", "new_password": "newpassword"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 200 + assert response.json()["status"] == "OK" + + # old password no longer works + assert _signin(client, "alice", "oldpassword").status_code == 401 + # new password works + assert _signin(client, "alice", "newpassword").status_code == 200 + + # password hash was actually updated in the db + expected_hash = hashlib.sha256("newpassword".encode()).hexdigest() + assert _get_password_hash(db_session, "alice") == expected_hash + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_change_own_password_wrong_current_password(db_session, client, test_app): + """Changing password fails if the current password is incorrect.""" + _create_db_user(db_session, "bob", "correctpassword") + + signin = _signin(client, "bob", "correctpassword") + assert signin.status_code == 200 + token = signin.json()["accessToken"] + + response = client.put( + "/auth/users/me/password", + json={"current_password": "wrongpassword", "new_password": "newpassword"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 403 + assert response.json()["detail"] == "Current password is incorrect" + + # password unchanged + assert _signin(client, "bob", "correctpassword").status_code == 200 + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_change_own_password_empty_new_password(db_session, client, test_app): + """Changing password fails if the new password is empty.""" + _create_db_user(db_session, "carol", "somepassword") + + signin = _signin(client, "carol", "somepassword") + assert signin.status_code == 200 + token = signin.json()["accessToken"] + + response = client.put( + "/auth/users/me/password", + json={"current_password": "somepassword", "new_password": " "}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 422 + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_change_password_requires_authentication(db_session, client, test_app): + """Changing password requires authentication.""" + response = client.put( + "/auth/users/me/password", + json={"current_password": "x", "new_password": "y"}, + ) + assert response.status_code == 401 + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_noc_user_can_change_own_password(db_session, client, test_app): + """A non-admin (noc) local user can still change their own password.""" + _create_db_user(db_session, "dave", "oldpass", role="noc") + + signin = _signin(client, "dave", "oldpass") + assert signin.status_code == 200 + token = signin.json()["accessToken"] + + response = client.put( + "/auth/users/me/password", + json={"current_password": "oldpass", "new_password": "brandnewpass"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 200 + assert _signin(client, "dave", "brandnewpass").status_code == 200 + + +@pytest.mark.parametrize( + "test_app", + [{"AUTH_TYPE": "DB", "KEEP_JWT_SECRET": "somesecret"}], + indirect=True, +) +def test_admin_can_reset_user_password_via_update(db_session, client, test_app): + """An admin can reset another user's password via the update endpoint.""" + _create_db_user(db_session, "admin_user", "adminpass", role="admin") + _create_db_user(db_session, "managed_user", "initialpass", role="noc") + + signin = _signin(client, "admin_user", "adminpass") + assert signin.status_code == 200 + token = signin.json()["accessToken"] + + response = client.put( + "/auth/users/managed_user", + json={"password": "resetpass"}, + headers={"Authorization": f"Bearer {token}"}, + ) + assert response.status_code == 200 + + # managed_user can sign in with new password + assert _signin(client, "managed_user", "resetpass").status_code == 200 + assert _signin(client, "managed_user", "initialpass").status_code == 401