From ae6839dc4286fbede1f88ea1df19da5bb914712b Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Fri, 29 Aug 2025 13:41:55 -0700 Subject: [PATCH 01/12] implementation as discussed with team This re implements commit 0ffb56145a84fca7b13bb51fa603c2d883250703 Thanks Eliana and Tom --- CHANGELOG.md | 4 +- .../user-management/NewPasswordModal.tsx | 19 ++++ .../user-management/UpdatePasswordModal.tsx | 16 ++++ .../api/api/v1/endpoints/user_endpoints.py | 23 +++++ src/fides/api/models/client.py | 14 +-- src/fides/api/models/fides_user.py | 3 +- src/fides/api/oauth/utils.py | 34 +++++++- .../api/v1/endpoints/test_user_endpoints.py | 86 +++++++++++++++++-- 8 files changed, 180 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 285348b953f..20222d06b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,13 +56,11 @@ 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) ### 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) - 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) ## [2.68.0](https://github.com/ethyca/fides/compare/2.67.2...2.68.0) diff --git a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx index f763ce9db29..79c03ea35e8 100644 --- a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx @@ -13,8 +13,13 @@ 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 { LOGIN_ROUTE, STORAGE_ROOT_KEY } from "~/constants"; +import { logout, 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"; @@ -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,17 @@ const useNewPasswordModal = (id: string) => { ), ); modal.onClose(); + + // Only logout if admin reset their own password + if (currentUser?.id === id) { + try { + localStorage.removeItem(STORAGE_ROOT_KEY); + } catch (e) { + // no-op + } + dispatch(logout()); + router.push(LOGIN_ROUTE); + } } }; diff --git a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx index 0af58e45040..511eadbdd37 100644 --- a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx @@ -13,7 +13,12 @@ import { useDisclosure, useToast, } from "fidesui"; +import { useRouter } from "next/router"; import React, { useState } from "react"; +import { useDispatch } from "react-redux"; + +import { LOGIN_ROUTE, STORAGE_ROOT_KEY } from "~/constants"; +import { logout } from "~/features/auth/auth.slice"; import { successToastParams } from "../common/toast"; import { useUpdateUserPasswordMutation } from "./user-management.slice"; @@ -24,6 +29,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 +56,16 @@ const useUpdatePasswordModal = (id: string) => { .unwrap() .then(() => { toast(successToastParams("Password updated")); + // Clear persisted auth state and logout locally + try { + localStorage.removeItem(STORAGE_ROOT_KEY); + } catch (e) { + // no-op + } + dispatch(logout()); modal.onClose(); + // Redirect to login + 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 35aa78d8e3d..5f7ee639c7f 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..76502124787 100644 --- a/src/fides/api/models/client.py +++ b/src/fides/api/models/client.py @@ -2,7 +2,7 @@ import json from datetime import datetime -from typing import Any, Optional +from typing import TYPE_CHECKING, Any, Optional from sqlalchemy import ARRAY, Column, ForeignKey, String from sqlalchemy.ext.declarative import declared_attr @@ -22,10 +22,12 @@ JWE_PAYLOAD_SYSTEMS, ) from fides.api.db.base_class import Base -from fides.api.models.fides_user import FidesUser from fides.api.oauth.jwt import generate_jwe from fides.config import FidesConfig +if TYPE_CHECKING: + from fides.api.models.fides_user import FidesUser + DEFAULT_SCOPES: list[str] = [] DEFAULT_ROLES: list[str] = [] DEFAULT_SYSTEMS: list[str] = [] @@ -48,9 +50,11 @@ def __tablename__(self) -> str: ARRAY(String), nullable=False, server_default="{}", default=dict ) fides_key = Column(String, index=True, unique=True, nullable=True) - user_id = Column( - String, ForeignKey(FidesUser.id_field_path), nullable=True, unique=True - ) + user_id = Column(String, ForeignKey("fidesuser.id"), nullable=True, unique=True) + + if TYPE_CHECKING: + # Explicitly annotate the backref relationship for mypy + user: Optional["FidesUser"] @classmethod def create_client_and_secret( diff --git a/src/fides/api/models/fides_user.py b/src/fides/api/models/fides_user.py index 36b93e44b11..77cd6b992ea 100644 --- a/src/fides/api/models/fides_user.py +++ b/src/fides/api/models/fides_user.py @@ -22,7 +22,7 @@ from fides.api.db.base_class import Base from fides.api.models.audit_log import AuditLog -# Intentionally importing SystemManager here to build the FidesUser.systems relationship +# SystemManager import is handled in base.py to avoid circular imports from fides.api.schemas.user import DisabledReason from fides.config import CONFIG @@ -32,7 +32,6 @@ FidesUserRespondentEmailVerification, ) from fides.api.models.sql_models import System # type: ignore[attr-defined] - from fides.api.models.system_manager import SystemManager class FidesUser(Base): diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index 5e3862ccd07..343476ac165 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, Security from fastapi.security import SecurityScopes @@ -80,6 +80,26 @@ 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: + password_reset_at = client.user.password_reset_at + if password_reset_at and issued_at < 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 +245,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 +390,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 +416,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..fbcb3e249fd 100644 --- a/tests/ops/api/v1/endpoints/test_user_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_endpoints.py @@ -1222,6 +1222,43 @@ 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_force_update_different_user_password_without_scope( self, api_client, @@ -1960,7 +1997,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 +2032,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 +2116,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 +2130,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 From 8a6ccb004cfc27e0c894afa668144616b7f85f78 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Fri, 29 Aug 2025 13:48:53 -0700 Subject: [PATCH 02/12] revert changes that removed SystemManager import --- src/fides/api/models/fides_user.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fides/api/models/fides_user.py b/src/fides/api/models/fides_user.py index 77cd6b992ea..36b93e44b11 100644 --- a/src/fides/api/models/fides_user.py +++ b/src/fides/api/models/fides_user.py @@ -22,7 +22,7 @@ from fides.api.db.base_class import Base from fides.api.models.audit_log import AuditLog -# SystemManager import is handled in base.py to avoid circular imports +# Intentionally importing SystemManager here to build the FidesUser.systems relationship from fides.api.schemas.user import DisabledReason from fides.config import CONFIG @@ -32,6 +32,7 @@ FidesUserRespondentEmailVerification, ) from fides.api.models.sql_models import System # type: ignore[attr-defined] + from fides.api.models.system_manager import SystemManager class FidesUser(Base): From 8b40e8fbd6758b7cbd26c714fdd34d197e2a0f31 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Fri, 29 Aug 2025 14:45:27 -0700 Subject: [PATCH 03/12] simplify password reset check --- src/fides/api/oauth/utils.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index 343476ac165..43c8f7b27dd 100644 --- a/src/fides/api/oauth/utils.py +++ b/src/fides/api/oauth/utils.py @@ -88,10 +88,12 @@ def is_token_invalidated(issued_at: datetime, client: ClientDetail) -> bool: 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: - password_reset_at = client.user.password_reset_at - if password_reset_at and issued_at < password_reset_at: - return True + 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( From ab91010f72b1bdf639f2992caa4718a490f574bc Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Fri, 29 Aug 2025 14:52:16 -0700 Subject: [PATCH 04/12] restore using constant id_field_path for FK identifier --- src/fides/api/models/client.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/fides/api/models/client.py b/src/fides/api/models/client.py index 76502124787..d84d8b64c41 100644 --- a/src/fides/api/models/client.py +++ b/src/fides/api/models/client.py @@ -2,7 +2,7 @@ import json from datetime import datetime -from typing import TYPE_CHECKING, Any, Optional +from typing import Any, Optional from sqlalchemy import ARRAY, Column, ForeignKey, String from sqlalchemy.ext.declarative import declared_attr @@ -22,12 +22,10 @@ JWE_PAYLOAD_SYSTEMS, ) from fides.api.db.base_class import Base +from fides.api.models.fides_user import FidesUser from fides.api.oauth.jwt import generate_jwe from fides.config import FidesConfig -if TYPE_CHECKING: - from fides.api.models.fides_user import FidesUser - DEFAULT_SCOPES: list[str] = [] DEFAULT_ROLES: list[str] = [] DEFAULT_SYSTEMS: list[str] = [] @@ -50,11 +48,10 @@ def __tablename__(self) -> str: ARRAY(String), nullable=False, server_default="{}", default=dict ) fides_key = Column(String, index=True, unique=True, nullable=True) - user_id = Column(String, ForeignKey("fidesuser.id"), nullable=True, unique=True) - - if TYPE_CHECKING: - # Explicitly annotate the backref relationship for mypy - user: Optional["FidesUser"] + user_id = Column( + String, ForeignKey(FidesUser.id_field_path), nullable=True, unique=True + ) + user: Optional["FidesUser"] @classmethod def create_client_and_secret( From 39e67dfc9a6f2f4d276d74f547784cbf0fa635c9 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Fri, 29 Aug 2025 15:39:45 -0700 Subject: [PATCH 05/12] add additional code coverage --- .../api/v1/endpoints/test_user_endpoints.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/ops/api/v1/endpoints/test_user_endpoints.py b/tests/ops/api/v1/endpoints/test_user_endpoints.py index fbcb3e249fd..dc26e137888 100644 --- a/tests/ops/api/v1/endpoints/test_user_endpoints.py +++ b/tests/ops/api/v1/endpoints/test_user_endpoints.py @@ -1259,6 +1259,35 @@ def test_token_invalid_after_password_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, @@ -1316,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", [ From 421be70f4863efa507a987eaa4119eec5a2b4475 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Fri, 29 Aug 2025 19:55:30 -0700 Subject: [PATCH 06/12] fix - for code coverage and testability --- src/fides/api/oauth/utils.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index 43c8f7b27dd..77b070e402a 100644 --- a/src/fides/api/oauth/utils.py +++ b/src/fides/api/oauth/utils.py @@ -88,13 +88,18 @@ def is_token_invalidated(issued_at: datetime, client: ClientDetail) -> bool: 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 + if client.user is None or client.user.password_reset_at is None: + return False + + reset_at = client.user.password_reset_at + # DB layer may return a tz-aware timestamp while the JWE issued_at is naive (server time). + # Normalize both to naive for a safe, deterministic comparison without raising TypeError. + issued_naive = issued_at.replace(tzinfo=None) + reset_naive = ( + reset_at.replace(tzinfo=None) if reset_at.tzinfo is not None else reset_at + ) + + return issued_naive < reset_naive except Exception as exc: logger.exception( "Unable to evaluate password reset timestamp for client user: {}", exc From 0a5d74ee7275cc3b737bb8390f82a1a78407e884 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Tue, 2 Sep 2025 11:56:15 -0700 Subject: [PATCH 07/12] update to use shared logout function and avoid duplication --- .../user-management/NewPasswordModal.tsx | 11 +++-------- .../user-management/UpdatePasswordModal.tsx | 15 +++------------ .../user-management/logout-helpers.ts | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 20 deletions(-) create mode 100644 clients/admin-ui/src/features/user-management/logout-helpers.ts diff --git a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx index 79c03ea35e8..e3fc2505b86 100644 --- a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx @@ -18,8 +18,9 @@ import { useDispatch } from "react-redux"; import * as Yup from "yup"; import { useAppSelector } from "~/app/hooks"; -import { LOGIN_ROUTE, STORAGE_ROOT_KEY } from "~/constants"; +import { LOGIN_ROUTE } from "~/constants"; import { logout, selectUser } from "~/features/auth/auth.slice"; +import { clearAuthAndLogout } from "./logout-helpers"; import { CustomTextInput } from "~/features/common/form/inputs"; import { passwordValidation } from "~/features/common/form/validation"; import { getErrorMessage } from "~/features/common/helpers"; @@ -63,13 +64,7 @@ const useNewPasswordModal = (id: string) => { // Only logout if admin reset their own password if (currentUser?.id === id) { - try { - localStorage.removeItem(STORAGE_ROOT_KEY); - } catch (e) { - // no-op - } - dispatch(logout()); - router.push(LOGIN_ROUTE); + 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 511eadbdd37..e075b02a84a 100644 --- a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx @@ -17,8 +17,8 @@ import { useRouter } from "next/router"; import React, { useState } from "react"; import { useDispatch } from "react-redux"; -import { LOGIN_ROUTE, STORAGE_ROOT_KEY } from "~/constants"; -import { logout } from "~/features/auth/auth.slice"; +import { LOGIN_ROUTE } from "~/constants"; +import { clearAuthAndLogout } from "./logout-helpers"; import { successToastParams } from "../common/toast"; import { useUpdateUserPasswordMutation } from "./user-management.slice"; @@ -56,16 +56,7 @@ const useUpdatePasswordModal = (id: string) => { .unwrap() .then(() => { toast(successToastParams("Password updated")); - // Clear persisted auth state and logout locally - try { - localStorage.removeItem(STORAGE_ROOT_KEY); - } catch (e) { - // no-op - } - dispatch(logout()); - modal.onClose(); - // Redirect to login - router.push(LOGIN_ROUTE); + 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..42e1f193bee --- /dev/null +++ b/clients/admin-ui/src/features/user-management/logout-helpers.ts @@ -0,0 +1,19 @@ +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); +}; From 69b7b019391e4e821c8443bfd3e8155598fc4798 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Tue, 2 Sep 2025 12:05:55 -0700 Subject: [PATCH 08/12] security items moved out of 2.69.0 release --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc607450580..ea718456ca0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ 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) +- 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) + ### Deprecated - DSR 2.0 is deprecated. New requests will be created using DSR 3.0 only. Existing DSR 2.0 requests will continue to process until completion. [#6458](https://github.com/ethyca/fides/pull/6458) @@ -62,11 +67,6 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o - 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) -### 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) -- 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) - ## [2.68.0](https://github.com/ethyca/fides/compare/2.67.2...2.68.0) ### Added From fca18e9abb326648c183196cbe725652f4ab31e9 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Tue, 2 Sep 2025 13:03:47 -0700 Subject: [PATCH 09/12] make tz removal consistent between issued_at and reset_at --- src/fides/api/oauth/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index 77b070e402a..c05efd07a32 100644 --- a/src/fides/api/oauth/utils.py +++ b/src/fides/api/oauth/utils.py @@ -94,7 +94,9 @@ def is_token_invalidated(issued_at: datetime, client: ClientDetail) -> bool: reset_at = client.user.password_reset_at # DB layer may return a tz-aware timestamp while the JWE issued_at is naive (server time). # Normalize both to naive for a safe, deterministic comparison without raising TypeError. - issued_naive = issued_at.replace(tzinfo=None) + issued_naive = ( + issued_at.replace(tzinfo=None) if issued_at.tzinfo is not None else issued_at + ) reset_naive = ( reset_at.replace(tzinfo=None) if reset_at.tzinfo is not None else reset_at ) From 1b7b412634b7b89ac0fa64ee4bd32bd1c165c08f Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Tue, 2 Sep 2025 13:49:04 -0700 Subject: [PATCH 10/12] fix lints --- .../src/features/user-management/NewPasswordModal.tsx | 5 ++--- .../src/features/user-management/UpdatePasswordModal.tsx | 8 ++++---- .../src/features/user-management/logout-helpers.ts | 2 +- src/fides/api/oauth/utils.py | 4 +++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx index e3fc2505b86..25d9690e045 100644 --- a/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/NewPasswordModal.tsx @@ -18,15 +18,14 @@ import { useDispatch } from "react-redux"; import * as Yup from "yup"; import { useAppSelector } from "~/app/hooks"; -import { LOGIN_ROUTE } from "~/constants"; -import { logout, selectUser } from "~/features/auth/auth.slice"; -import { clearAuthAndLogout } from "./logout-helpers"; +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({ diff --git a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx index e075b02a84a..e82dcc46537 100644 --- a/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx +++ b/clients/admin-ui/src/features/user-management/UpdatePasswordModal.tsx @@ -17,10 +17,8 @@ import { useRouter } from "next/router"; import React, { useState } from "react"; import { useDispatch } from "react-redux"; -import { LOGIN_ROUTE } from "~/constants"; -import { clearAuthAndLogout } from "./logout-helpers"; - import { successToastParams } from "../common/toast"; +import { clearAuthAndLogout } from "./logout-helpers"; import { useUpdateUserPasswordMutation } from "./user-management.slice"; const useUpdatePasswordModal = (id: string) => { @@ -56,7 +54,9 @@ const useUpdatePasswordModal = (id: string) => { .unwrap() .then(() => { toast(successToastParams("Password updated")); - clearAuthAndLogout(dispatch as any, router, { onClose: 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 index 42e1f193bee..bba3c5c5fbb 100644 --- a/clients/admin-ui/src/features/user-management/logout-helpers.ts +++ b/clients/admin-ui/src/features/user-management/logout-helpers.ts @@ -6,7 +6,7 @@ import { logout } from "~/features/auth/auth.slice"; export const clearAuthAndLogout = ( dispatch: AppDispatch, router: NextRouter, - opts?: { onClose?: () => void } + opts?: { onClose?: () => void }, ) => { try { localStorage.removeItem(STORAGE_ROOT_KEY); diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index c05efd07a32..7ec53cb1152 100644 --- a/src/fides/api/oauth/utils.py +++ b/src/fides/api/oauth/utils.py @@ -95,7 +95,9 @@ def is_token_invalidated(issued_at: datetime, client: ClientDetail) -> bool: # DB layer may return a tz-aware timestamp while the JWE issued_at is naive (server time). # Normalize both to naive for a safe, deterministic comparison without raising TypeError. issued_naive = ( - issued_at.replace(tzinfo=None) if issued_at.tzinfo is not None else issued_at + issued_at.replace(tzinfo=None) + if issued_at.tzinfo is not None + else issued_at ) reset_naive = ( reset_at.replace(tzinfo=None) if reset_at.tzinfo is not None else reset_at From 67076e7dc15c97d7a18ea310cb4a5e17aca1fca6 Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Tue, 2 Sep 2025 14:30:20 -0700 Subject: [PATCH 11/12] another missed linter issue --- clients/admin-ui/src/features/user-management/logout-helpers.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/admin-ui/src/features/user-management/logout-helpers.ts b/clients/admin-ui/src/features/user-management/logout-helpers.ts index bba3c5c5fbb..191588f283a 100644 --- a/clients/admin-ui/src/features/user-management/logout-helpers.ts +++ b/clients/admin-ui/src/features/user-management/logout-helpers.ts @@ -1,4 +1,5 @@ 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"; From 6720ec28c202d4fea2303a4ed7b3424f45c44beb Mon Sep 17 00:00:00 2001 From: Thabo Fletcher Date: Tue, 2 Sep 2025 16:13:41 -0700 Subject: [PATCH 12/12] remove the terrible tzinfo normalization code --- src/fides/api/oauth/utils.py | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/fides/api/oauth/utils.py b/src/fides/api/oauth/utils.py index 192737a4d02..c111623b1e7 100644 --- a/src/fides/api/oauth/utils.py +++ b/src/fides/api/oauth/utils.py @@ -88,22 +88,13 @@ def is_token_invalidated(issued_at: datetime, client: ClientDetail) -> bool: Any errors accessing related objects are logged and treated as non-invalidating. """ try: - if client.user is None or client.user.password_reset_at is None: - return False - - reset_at = client.user.password_reset_at - # DB layer may return a tz-aware timestamp while the JWE issued_at is naive (server time). - # Normalize both to naive for a safe, deterministic comparison without raising TypeError. - issued_naive = ( - issued_at.replace(tzinfo=None) - if issued_at.tzinfo is not None - else issued_at - ) - reset_naive = ( - reset_at.replace(tzinfo=None) if reset_at.tzinfo is not None else reset_at - ) - - return issued_naive < reset_naive + 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