From bba8d6cdb84f13273621ab3a463e46954970d62b Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 13 Dec 2024 10:39:59 +0100 Subject: [PATCH] Improve auto logout --- .../src/layout/navigation/ProjectNotice.tsx | 34 ++++++++++--- posthog/middleware.py | 12 ++++- posthog/settings/web.py | 8 ++++ posthog/test/test_middleware.py | 48 +++++++++++++++++-- 4 files changed, 89 insertions(+), 13 deletions(-) diff --git a/frontend/src/layout/navigation/ProjectNotice.tsx b/frontend/src/layout/navigation/ProjectNotice.tsx index 5a0a9c4dd28ce..8d4df0246a3a0 100644 --- a/frontend/src/layout/navigation/ProjectNotice.tsx +++ b/frontend/src/layout/navigation/ProjectNotice.tsx @@ -1,4 +1,5 @@ import { IconGear, IconPlus } from '@posthog/icons' +import { Spinner } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' import { dayjs } from 'lib/dayjs' import { LemonBanner } from 'lib/lemon-ui/LemonBanner' @@ -22,17 +23,29 @@ interface ProjectNoticeBlueprint { closeable?: boolean } -function CountDown({ datetime }: { datetime: dayjs.Dayjs }): JSX.Element { +function CountDown({ datetime, callback }: { datetime: dayjs.Dayjs; callback?: () => void }): JSX.Element { const [now, setNow] = useState(dayjs()) + // Format the time difference as 00:00:00 + const duration = dayjs.duration(datetime.diff(now)) + const pastCountdown = duration.seconds() < 0 + + const countdown = pastCountdown + ? 'Expired' + : duration.hours() > 0 + ? duration.format('HH:mm:ss') + : duration.format('mm:ss') + useEffect(() => { const interval = setInterval(() => setNow(dayjs()), 1000) return () => clearInterval(interval) }, []) - // Format the time difference as 00:00:00 - const duration = dayjs.duration(datetime.diff(now)) - const countdown = duration.hours() > 0 ? duration.format('HH:mm:ss') : duration.format('mm:ss') + useEffect(() => { + if (pastCountdown) { + callback?.() + } + }, [pastCountdown]) return <>{countdown} } @@ -40,8 +53,8 @@ function CountDown({ datetime }: { datetime: dayjs.Dayjs }): JSX.Element { export function ProjectNotice(): JSX.Element | null { const { projectNoticeVariant } = useValues(navigationLogic) const { currentOrganization } = useValues(organizationLogic) - const { logout } = useActions(userLogic) - const { user } = useValues(userLogic) + const { logout, loadUser } = useActions(userLogic) + const { user, userLoading } = useValues(userLogic) const { closeProjectNotice } = useActions(navigationLogic) const { showInviteModal } = useActions(inviteLogic) const { requestVerificationLink } = useActions(verifyEmailLogic) @@ -124,7 +137,14 @@ export function ProjectNotice(): JSX.Element | null { You are currently logged in as a customer.{' '} {user?.is_impersonated_until && ( <> - Expires in + Expires in + {userLoading ? ( + + ) : ( + loadUser()}> + Refresh + + )} )} diff --git a/posthog/middleware.py b/posthog/middleware.py index ee132dc78d0af..57b561f27600d 100644 --- a/posthog/middleware.py +++ b/posthog/middleware.py @@ -677,7 +677,17 @@ def get_impersonated_session_expires_at(request: HttpRequest) -> Optional[dateti init_time = get_or_set_session_cookie_created_at(request=request) - return datetime.fromtimestamp(init_time) + timedelta(seconds=settings.IMPERSONATION_TIMEOUT_SECONDS) + last_activity_time = request.session.get(settings.IMPERSONATION_COOKIE_LAST_ACTIVITY_KEY, init_time) + + # If the last activity time is less than the idle timeout, we extend the session + if time.time() - last_activity_time < settings.IMPERSONATION_IDLE_TIMEOUT_SECONDS: + last_activity_time = request.session[settings.IMPERSONATION_COOKIE_LAST_ACTIVITY_KEY] = time.time() + request.session.modified = True + else: + # If the idle timeout has passed then we return it instead of the total timeout + return datetime.fromtimestamp(init_time) + + return datetime.fromtimestamp(last_activity_time) + timedelta(seconds=settings.IMPERSONATION_IDLE_TIMEOUT_SECONDS) class AutoLogoutImpersonateMiddleware: diff --git a/posthog/settings/web.py b/posthog/settings/web.py index 98b01dac20b2c..a2158084d52ad 100644 --- a/posthog/settings/web.py +++ b/posthog/settings/web.py @@ -376,7 +376,15 @@ # Used only to display in the UI to inform users of allowlist options PUBLIC_EGRESS_IP_ADDRESSES = get_list(os.getenv("PUBLIC_EGRESS_IP_ADDRESSES", "")) +# The total time allowed for an impersonated session IMPERSONATION_TIMEOUT_SECONDS = get_from_env("IMPERSONATION_TIMEOUT_SECONDS", 15 * 60, type_cast=int) +# The time allowed for an impersonated session to be idle before it expires +IMPERSONATION_IDLE_TIMEOUT_SECONDS = get_from_env("IMPERSONATION_IDLE_TIMEOUT_SECONDS", 15 * 60, type_cast=int) +# Impersonation cookie last activity key +IMPERSONATION_COOKIE_LAST_ACTIVITY_KEY = get_from_env( + "IMPERSONATION_COOKIE_LAST_ACTIVITY_KEY", "impersonation_last_activity" +) + SESSION_COOKIE_CREATED_AT_KEY = get_from_env("SESSION_COOKIE_CREATED_AT_KEY", "session_created_at") PROJECT_SWITCHING_TOKEN_ALLOWLIST = get_list(os.getenv("PROJECT_SWITCHING_TOKEN_ALLOWLIST", "sTMFPsFhdP1Ssg")) diff --git a/posthog/test/test_middleware.py b/posthog/test/test_middleware.py index e6a9e95ac9ba0..d952676eb2514 100644 --- a/posthog/test/test_middleware.py +++ b/posthog/test/test_middleware.py @@ -501,7 +501,8 @@ def test_logout(self): self.assertNotIn("ph_current_project_name", response.cookies) -@override_settings(IMPERSONATION_TIMEOUT_SECONDS=30) +@override_settings(IMPERSONATION_TIMEOUT_SECONDS=120) +@override_settings(IMPERSONATION_IDLE_TIMEOUT_SECONDS=30) class TestAutoLogoutImpersonateMiddleware(APIBaseTest): other_user: User @@ -538,21 +539,58 @@ def test_not_staff_user_cannot_login(self): assert response.status_code == 200 assert self.client.get("/api/users/@me").json()["email"] == self.user.email - def test_after_timeout_api_requests_401(self): - now = datetime.now() + def test_after_idle_timeout_api_requests_401(self): + now = datetime(2024, 1, 1, 12, 0, 0) with freeze_time(now): self.login_as_other_user() res = self.client.get("/api/users/@me") assert res.status_code == 200 assert res.json()["email"] == "other-user@posthog.com" + assert res.json()["is_impersonated_until"] == "2024-01-01T12:00:30+00:00" assert self.client.session.get("session_created_at") == now.timestamp() - with freeze_time(now + timedelta(seconds=10)): + # Move forward by 20 + now = now + timedelta(seconds=20) + with freeze_time(now): res = self.client.get("/api/users/@me") assert res.status_code == 200 assert res.json()["email"] == "other-user@posthog.com" + assert res.json()["is_impersonated_until"] == "2024-01-01T12:00:50+00:00" - with freeze_time(now + timedelta(seconds=35)): + # Past idle timeout + now = now + timedelta(seconds=35) + + with freeze_time(now): + res = self.client.get("/api/users/@me") + assert res.status_code == 401 + + def test_after_total_timeout_api_requests_401(self): + now = datetime(2024, 1, 1, 12, 0, 0) + with freeze_time(now): + self.login_as_other_user() + res = self.client.get("/api/users/@me") + assert res.status_code == 200 + assert res.json()["email"] == "other-user@posthog.com" + assert res.json()["is_impersonated_until"] == "2024-01-01T12:00:30+00:00" + assert self.client.session.get("session_created_at") == now.timestamp() + + for _ in range(5): + # Move forward by 20 seconds 5 times + now = now + timedelta(seconds=20) + with freeze_time(now): + res = self.client.get("/api/users/@me") + assert res.status_code == 200 + assert res.json()["email"] == "other-user@posthog.com" + # Format exactly like the date above + assert res.json()["is_impersonated_until"] == (now + timedelta(seconds=30)).strftime( + "%Y-%m-%dT%H:%M:%S+00:00" + ) + + now = now + timedelta( + seconds=25 + ) # Final time takes us past the total timeout, despite being under idle timeout + + with freeze_time(now): res = self.client.get("/api/users/@me") assert res.status_code == 401