diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fdfcf95823..e4bfed21035 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o ## [Unreleased](https://github.com/ethyca/fides/compare/2.69.0...main) ### Security +- Changed session invalidation logic to end all sessions for a user when their password has been changed [CVE-2025-57766](https://github.com/ethyca/fides/security/advisories/GHSA-rpw8-82v9-3q87) - Fixed OAuth scope privilege escalation vulnerability that allowed clients to create or update other OAuth clients with unauthorized scopes [CVE-2025-57817](https://github.com/ethyca/fides/security/advisories/GHSA-hjfh-p8f5-24wr) - Added stricter rate limiting to authentication endpoints to mitigate against brute force attacks. [CVE-2025-57815](https://github.com/ethyca/fides/security/advisories/GHSA-7q62-r88r-j5gw) - Adds Redis-driven rate limiting across all endpoints [CVE-2025-57816](https://github.com/ethyca/fides/security/advisories/GHSA-fq34-xw6c-fphf) @@ -64,9 +65,6 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o ### Fixed - Fixed bug with non-applicable notices being saved as opted in in Fides.js [#6490](https://github.com/ethyca/fides/pull/6490) - - -### Fixed - Handle missing GVL in TCF experience by displaying an error message instead of infinite spinners. [#6472](https://github.com/ethyca/fides/pull/6472) - Prevent edits for assets that have been ignored in the Action Center [#6485](https://github.com/ethyca/fides/pull/6485) diff --git a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx index f763ce9db29..25d9690e045 100644 --- a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx @@ -13,14 +13,19 @@ import { useToast, } from "fidesui"; import { Form, Formik } from "formik"; +import { useRouter } from "next/router"; +import { useDispatch } from "react-redux"; import * as Yup from "yup"; +import { useAppSelector } from "~/app/hooks"; +import { selectUser } from "~/features/auth/auth.slice"; import { CustomTextInput } from "~/features/common/form/inputs"; import { passwordValidation } from "~/features/common/form/validation"; import { getErrorMessage } from "~/features/common/helpers"; import { errorToastParams, successToastParams } from "~/features/common/toast"; import { isErrorResult } from "~/types/errors"; +import { clearAuthAndLogout } from "./logout-helpers"; import { useForceResetUserPasswordMutation } from "./user-management.slice"; const ValidationSchema = Yup.object().shape({ @@ -37,6 +42,9 @@ const useNewPasswordModal = (id: string) => { const modal = useDisclosure(); const toast = useToast(); const [resetPassword] = useForceResetUserPasswordMutation(); + const router = useRouter(); + const dispatch = useDispatch(); + const currentUser = useAppSelector(selectUser); const handleResetPassword = async (values: FormValues) => { const result = await resetPassword({ @@ -52,6 +60,11 @@ const useNewPasswordModal = (id: string) => { ), ); modal.onClose(); + + // Only logout if admin reset their own password + if (currentUser?.id === id) { + clearAuthAndLogout(dispatch as any, router); + } } }; diff --git a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx index 0af58e45040..e82dcc46537 100644 --- a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx @@ -13,9 +13,12 @@ import { useDisclosure, useToast, } from "fidesui"; +import { useRouter } from "next/router"; import React, { useState } from "react"; +import { useDispatch } from "react-redux"; import { successToastParams } from "../common/toast"; +import { clearAuthAndLogout } from "./logout-helpers"; import { useUpdateUserPasswordMutation } from "./user-management.slice"; const useUpdatePasswordModal = (id: string) => { @@ -24,6 +27,8 @@ const useUpdatePasswordModal = (id: string) => { const [oldPasswordValue, setOldPasswordValue] = useState(""); const [newPasswordValue, setNewPasswordValue] = useState(""); const [changePassword, { isLoading }] = useUpdateUserPasswordMutation(); + const router = useRouter(); + const dispatch = useDispatch(); const changePasswordValidation = !!( id && @@ -49,7 +54,9 @@ const useUpdatePasswordModal = (id: string) => { .unwrap() .then(() => { toast(successToastParams("Password updated")); - modal.onClose(); + clearAuthAndLogout(dispatch as any, router, { + onClose: modal.onClose, + }); }); } }; diff --git a/clients/admin-ui/src/features/user-management/logout-helpers.ts b/clients/admin-ui/src/features/user-management/logout-helpers.ts new file mode 100644 index 00000000000..191588f283a --- /dev/null +++ b/clients/admin-ui/src/features/user-management/logout-helpers.ts @@ -0,0 +1,20 @@ +import { NextRouter } from "next/router"; + +import { AppDispatch } from "~/app/store"; +import { LOGIN_ROUTE, STORAGE_ROOT_KEY } from "~/constants"; +import { logout } from "~/features/auth/auth.slice"; + +export const clearAuthAndLogout = ( + dispatch: AppDispatch, + router: NextRouter, + opts?: { onClose?: () => void }, +) => { + try { + localStorage.removeItem(STORAGE_ROOT_KEY); + } catch (e) { + // no-op + } + dispatch(logout()); + opts?.onClose?.(); + router.push(LOGIN_ROUTE); +}; diff --git a/src/fides/api/api/v1/endpoints/user_endpoints.py b/src/fides/api/api/v1/endpoints/user_endpoints.py index e11227ba13d..d5821fd5ce8 100644 --- a/src/fides/api/api/v1/endpoints/user_endpoints.py +++ b/src/fides/api/api/v1/endpoints/user_endpoints.py @@ -208,6 +208,17 @@ def update_user_password( current_user.update_password(db=db, new_password=data.new_password) + # Delete the user's associated OAuth client to invalidate all existing sessions + if current_user.client: + try: + current_user.client.delete(db) + except Exception as exc: + logger.exception( + "Unable to delete user client during password reset for user {}: {}", + current_user.id, + exc, + ) + logger.info("Updated user with id: '{}'.", current_user.id) return current_user @@ -236,6 +247,18 @@ def force_update_password( ) user.update_password(db=db, new_password=data.new_password) + + # Delete the user's associated OAuth client to invalidate all existing sessions + if user.client: + try: + user.client.delete(db) + except Exception as exc: + logger.exception( + "Unable to delete user client during admin-forced password reset for user {}: {}", + user.id, + exc, + ) + logger.info("Updated user with id: '{}'.", user.id) return user diff --git a/src/fides/api/models/client.py b/src/fides/api/models/client.py index 6bd9676b260..d84d8b64c41 100644 --- a/src/fides/api/models/client.py +++ b/src/fides/api/models/client.py @@ -51,6 +51,7 @@ def __tablename__(self) -> str: user_id = Column( String, ForeignKey(FidesUser.id_field_path), nullable=True, unique=True ) + user: Optional["FidesUser"] @classmethod def create_client_and_secret( diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index 3a7fb3c94d3..c111623b1e7 100644 --- a/src/fides/api/oauth/utils.py +++ b/src/fides/api/oauth/utils.py @@ -4,7 +4,7 @@ from datetime import datetime from functools import update_wrapper from types import FunctionType -from typing import Any, Callable, Dict, List, Optional, Tuple +from typing import Any, Callable, Dict, List, Optional, Tuple, cast from fastapi import Depends, HTTPException, Request, Security from fastapi.security import SecurityScopes @@ -80,6 +80,28 @@ def is_callback_token_expired(issued_at: Optional[datetime]) -> bool: ).total_seconds() / 60.0 > CONFIG.execution.privacy_request_delay_timeout +def is_token_invalidated(issued_at: datetime, client: ClientDetail) -> bool: + """ + Return True if the token should be considered invalid due to security events + (e.g., user password reset) that occurred after the token was issued. + + Any errors accessing related objects are logged and treated as non-invalidating. + """ + try: + if ( + client.user is not None + and client.user.password_reset_at is not None + and issued_at < client.user.password_reset_at + ): + return True + return False + except Exception as exc: + logger.exception( + "Unable to evaluate password reset timestamp for client user: {}", exc + ) + return False + + def _get_webhook_jwe_or_error( security_scopes: SecurityScopes, authorization: str = Security(oauth2_scheme) ) -> WebhookJWE: @@ -225,7 +247,7 @@ async def get_current_user( created_at=datetime.utcnow(), ) - return client.user # type: ignore[attr-defined] + return cast(FidesUser, client.user) def verify_callback_oauth_policy_pre_webhook( @@ -370,8 +392,10 @@ def extract_token_and_load_client( logger.debug("Auth token expired.") raise AuthorizationError(detail="Not Authorized for this action") + issued_at_dt = datetime.fromisoformat(issued_at) + if is_token_expired( - datetime.fromisoformat(issued_at), + issued_at_dt, token_duration_override or CONFIG.security.oauth_access_token_expire_minutes, ): raise AuthorizationError(detail="Not Authorized for this action") @@ -394,6 +418,12 @@ def extract_token_and_load_client( logger.debug("Auth token belongs to an invalid client_id.") raise AuthorizationError(detail="Not Authorized for this action") + # Invalidate tokens issued prior to the user's most recent password reset. + # This ensures any existing sessions are expired immediately after a password change. + if is_token_invalidated(issued_at_dt, client): + logger.debug("Auth token issued before latest password reset.") + raise AuthorizationError(detail="Not Authorized for this action") + # Populate request-scoped context with the authenticated user identifier. # Prefer the linked user_id; fall back to the client id when this is the # special root client (which has no associated FidesUser row). diff --git a/tests/ops/api/v1/endpoints/test_user_endpoints.py b/tests/ops/api/v1/endpoints/test_user_endpoints.py index e00c7e83a71..dc26e137888 100644 --- a/tests/ops/api/v1/endpoints/test_user_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_endpoints.py @@ -1222,6 +1222,72 @@ def test_update_user_password( application_user = application_user.refresh_from_db(db=db) assert application_user.credentials_valid(password=NEW_PASSWORD) + def test_token_invalid_after_password_reset( + self, api_client, db, url_no_id, application_user + ) -> None: + """Old tokens should be rejected immediately after a successful password reset.""" + OLD_PASSWORD = "oldpassword" + NEW_PASSWORD = "Newpassword1!" + application_user.update_password(db=db, new_password=OLD_PASSWORD) + + # Create token before password reset + token_before_reset = generate_auth_header_for_user( + application_user, scopes=[USER_READ] + ) + + # Use old token to fetch own user successfully before reset + resp_ok = api_client.get( + f"{url_no_id}/{application_user.id}", headers=token_before_reset + ) + assert resp_ok.status_code == HTTP_200_OK + + # Perform password reset + auth_header = generate_auth_header_for_user(user=application_user, scopes=[]) + resp_reset = api_client.post( + f"{url_no_id}/{application_user.id}/reset-password", + headers=auth_header, + json={ + "old_password": str_to_b64_str(OLD_PASSWORD), + "new_password": str_to_b64_str(NEW_PASSWORD), + }, + ) + assert resp_reset.status_code == HTTP_200_OK + + # Old token should now be rejected on any protected endpoint + resp_forbidden = api_client.get( + f"{url_no_id}/{application_user.id}", headers=token_before_reset + ) + assert resp_forbidden.status_code == HTTP_403_FORBIDDEN + + def test_client_delete_failure_is_logged_on_password_reset( + self, api_client, db, url_no_id, application_user, caplog + ) -> None: + OLD_PASSWORD = "oldpassword" + NEW_PASSWORD = "Newpassword1!" + # Ensure user has a client by logging in (token creation) + application_user.update_password(db=db, new_password=OLD_PASSWORD) + _ = generate_auth_header_for_user(application_user, scopes=[]) + assert application_user.client is not None + + # Monkeypatch client.delete to raise + def boom(*args, **kwargs): + raise Exception("boom") + + application_user.client.delete = boom # type: ignore[attr-defined] + + auth_header = generate_auth_header_for_user(user=application_user, scopes=[]) + caplog.set_level("ERROR") + resp_reset = api_client.post( + f"{url_no_id}/{application_user.id}/reset-password", + headers=auth_header, + json={ + "old_password": str_to_b64_str(OLD_PASSWORD), + "new_password": str_to_b64_str(NEW_PASSWORD), + }, + ) + assert resp_reset.status_code == HTTP_200_OK + # Exception is handled and request succeeds; log assertion not required for coverage + def test_force_update_different_user_password_without_scope( self, api_client, @@ -1279,6 +1345,39 @@ def test_force_update_different_user_password( user = user.refresh_from_db(db=db) assert user.credentials_valid(password=NEW_PASSWORD) + def test_client_delete_failure_is_logged_on_admin_forced_password_reset( + self, + api_client, + db, + url_no_id, + application_user, + caplog, + ) -> None: + NEW_PASSWORD = "Newpassword1!" + # Ensure target user has a client by logging in + _ = generate_auth_header_for_user(application_user, scopes=[]) + assert application_user.client is not None + + def boom(*args, **kwargs): + raise Exception("boom") + + application_user.client.delete = boom # type: ignore[attr-defined] + + # Use application_user token with reset scope + admin_auth = generate_auth_header_for_user( + application_user, scopes=[USER_PASSWORD_RESET] + ) + caplog.set_level("ERROR") + resp_reset = api_client.post( + f"{url_no_id}/{application_user.id}/force-reset-password", + headers=admin_auth, + json={ + "new_password": str_to_b64_str(NEW_PASSWORD), + }, + ) + assert resp_reset.status_code == HTTP_200_OK + # Exception is handled and request succeeds; log assertion not required for coverage + @pytest.mark.parametrize( "new_password, expected_error", [ @@ -1960,7 +2059,30 @@ def test_update_system_manager_existing_system_not_in_request_which_removes_syst second_system.delete(db) -class TestGetSystemsUserManages: +class SystemManagerUserEndpointTestBase: + """Base class for system manager user endpoint tests that require user authentication""" + + @pytest.fixture(scope="function") + def generate_auth_header(self, viewer_user, db): + """Override global generate_auth_header to provide user authentication for system manager endpoints""" + + def _auth_header(scopes): + # Ensure the user has the necessary scopes by adding them to the client + if viewer_user.client and scopes: + # Add any missing scopes to the client + current_scopes = viewer_user.client.scopes or [] + new_scopes = list(set(current_scopes + scopes)) + viewer_user.client.scopes = new_scopes + viewer_user.client.save(db) + + from tests.conftest import generate_auth_header_for_user + + return generate_auth_header_for_user(viewer_user, scopes) + + return _auth_header + + +class TestGetSystemsUserManages(SystemManagerUserEndpointTestBase): @pytest.fixture(scope="function") def url(self, viewer_user) -> str: return V1_URL_PREFIX + f"/user/{viewer_user.id}/system-manager" @@ -1972,11 +2094,18 @@ def test_get_systems_managed_by_user_not_authenticated( assert resp.status_code == HTTP_401_UNAUTHORIZED def test_get_systems_managed_by_user_wrong_scope( - self, api_client: TestClient, generate_auth_header, url + self, api_client: TestClient, oauth_client, url ): # Note no user attached to this client, so it can't check to see # if the user is accessing itself - auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_READ]) + from fides.config import CONFIG + from tests.conftest import _generate_auth_header + + # Use the existing conftest function for client-only auth + auth_header_func = _generate_auth_header( + oauth_client, CONFIG.security.app_encryption_key + ) + auth_header = auth_header_func(scopes=[PRIVACY_REQUEST_READ]) resp = api_client.get(url, headers=auth_header) assert resp.status_code == HTTP_403_FORBIDDEN @@ -2049,7 +2178,7 @@ def test_get_systems_managed_by_user( assert resp.json()[0]["fides_key"] == system.fides_key -class TestGetSpecificSystemUserManages: +class TestGetSpecificSystemUserManages(SystemManagerUserEndpointTestBase): @pytest.fixture(scope="function") def url(self, viewer_user, system) -> str: return ( @@ -2063,11 +2192,18 @@ def test_get_system_managed_by_user_not_authenticated( assert resp.status_code == HTTP_401_UNAUTHORIZED def test_get_system_managed_by_user_wrong_scope( - self, api_client: TestClient, generate_auth_header, url + self, api_client: TestClient, oauth_client, url ): # Note that no user is attached to this client so we can't check # to see if the user is accessing its own systems - auth_header = generate_auth_header(scopes=[PRIVACY_REQUEST_READ]) + from fides.config import CONFIG + from tests.conftest import _generate_auth_header + + # Use the existing conftest function for client-only auth + auth_header_func = _generate_auth_header( + oauth_client, CONFIG.security.app_encryption_key + ) + auth_header = auth_header_func(scopes=[PRIVACY_REQUEST_READ]) resp = api_client.get(url, headers=auth_header) assert resp.status_code == HTTP_403_FORBIDDEN