-
Notifications
You must be signed in to change notification settings - Fork 3
Cdd 3363 fe bug logout warning countdown drifts from real session when tab is inactive #986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
9b19eab
6958b40
dee7893
0872aa8
592000d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,72 +16,101 @@ | |
| const [secondsLeft, setSecondsLeft] = useState(warningMinutes * 60) | ||
| const [visible, setVisible] = useState(false) | ||
|
|
||
| // The logoutAtRef contains real clock time (ms) when sign-out will happen. | ||
| // Reading Date.now() against this keeps the countdown correct, even | ||
| // when the browser pauses timers in a background tab when going to sleep. | ||
| const logoutAtRef = useRef<number>(0) | ||
| const inactivityTimer = useRef<ReturnType<typeof setTimeout> | null>(null) | ||
| const countdownInterval = useRef<ReturnType<typeof setInterval> | null>(null) | ||
| const visibleRef = useRef(false) | ||
|
|
||
| // Clear all timers and intervals | ||
| const clearTimers = useCallback(() => { | ||
| if (inactivityTimer.current) clearTimeout(inactivityTimer.current) | ||
| if (countdownInterval.current) clearInterval(countdownInterval.current) | ||
| inactivityTimer.current = null | ||
| countdownInterval.current = null | ||
| }, []) | ||
|
|
||
| // Trigger logout by clearing timers and calling the server sign out function | ||
| const triggerLogout = useCallback(async () => { | ||
| clearTimers() | ||
| serverSignOut('/logged-out') | ||
| }, [clearTimers]) | ||
|
|
||
| // Start the countdown when the warning is triggered | ||
| const startCountdown = useCallback(() => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this restart the full warning period when the tab becomes visible again? E.g, with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another good spot. Fixed. startCountdown() gets called from different places, so I am passing an optional argument now to get round this. |
||
| if (countdownInterval.current) clearInterval(countdownInterval.current) | ||
|
|
||
| logoutAtRef.current = Date.now() + warningMinutes * 60 * 1000 | ||
|
|
||
| setSecondsLeft(warningMinutes * 60) | ||
| setVisible(true) | ||
| visibleRef.current = true | ||
|
|
||
| countdownInterval.current = setInterval(() => { | ||
| setSecondsLeft((s) => { | ||
| if (s <= 1) { | ||
| clearInterval(countdownInterval.current!) | ||
| countdownInterval.current = null | ||
| triggerLogout() | ||
| return 0 | ||
| } | ||
| return s - 1 | ||
| }) | ||
| const remaining = Math.max(0, Math.ceil((logoutAtRef.current - Date.now()) / 1000)) | ||
| remaining === 0 ? triggerLogout() : setSecondsLeft(remaining) | ||
| }, 1000) | ||
| }, [warningMinutes, triggerLogout]) | ||
|
|
||
| // Schedule the warning to show after the appropriate amount of inactivity | ||
| const scheduleWarning = useCallback(() => { | ||
| if (inactivityTimer.current) clearTimeout(inactivityTimer.current) | ||
| const idleBeforeWarning = (timeoutMinutes - warningMinutes) * 60 * 1000 | ||
| inactivityTimer.current = setTimeout(startCountdown, idleBeforeWarning) | ||
| }, [timeoutMinutes, warningMinutes, startCountdown]) | ||
|
|
||
| // Reset timer on user activity, but only if warning isn't already visible | ||
| const resetInactivityTimer = useCallback(() => { | ||
| if (visibleRef.current) return | ||
| localStorage.setItem('lastActivity', Date.now().toString()) | ||
| scheduleWarning() | ||
| }, [scheduleWarning]) | ||
|
|
||
| // Set up event listeners for user activity and start the initial timer | ||
| useEffect(() => { | ||
| const events = ['mousemove', 'keydown', 'mousedown', 'touchstart', 'scroll'] | ||
| events.forEach((e) => window.addEventListener(e, resetInactivityTimer)) | ||
| localStorage.setItem('lastActivity', Date.now().toString()) | ||
| scheduleWarning() | ||
|
|
||
| // Fires the instant the user switches back to this tab. | ||
| // Corrects for time that passed while the browser slept and paused our timers. | ||
| const onVisibilityChange = () => { | ||
| if (document.hidden) return | ||
|
|
||
| if (visibleRef.current) { | ||
| // Modal is already showing, so snap the countdown to real remaining time. | ||
| // Example: User left with 1:45 min showing and was away 90 seconds, then now show 0:15. | ||
| const remaining = Math.max(0, Math.ceil((logoutAtRef.current - Date.now()) / 1000)) | ||
| remaining === 0 ? triggerLogout() : setSecondsLeft(remaining) | ||
| } else { | ||
| const lastActivity = Number(localStorage.getItem('lastActivity')) || Date.now() | ||
| const idleSeconds = (Date.now() - lastActivity) / 1000 | ||
|
|
||
| // Modal is not yet showing, so check how long the user has been idle | ||
| if (idleSeconds >= timeoutMinutes * 60) { | ||
| // Idle for longer than the full logout threshold, then log out now | ||
| triggerLogout() | ||
| } else if (idleSeconds >= (timeoutMinutes - warningMinutes) * 60) { | ||
| // Idle past the warning threshold but not the full logout threshold, then show modal. | ||
| startCountdown() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| window.addEventListener('visibilitychange', onVisibilityChange) | ||
|
Check warning on line 97 in src/app/components/ui/ukhsa/LogoutWarning/LogoutWarning.tsx
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good spot. Fixed. Worked for me before, but probably wasn't browser-compatible. |
||
|
|
||
| return () => { | ||
| events.forEach((e) => window.removeEventListener(e, resetInactivityTimer)) | ||
| window.removeEventListener('visibilitychange', onVisibilityChange) | ||
|
Check warning on line 101 in src/app/components/ui/ukhsa/LogoutWarning/LogoutWarning.tsx
|
||
| clearTimers() | ||
| } | ||
| }, [scheduleWarning, resetInactivityTimer, clearTimers]) | ||
| }, [ | ||
| scheduleWarning, | ||
| resetInactivityTimer, | ||
| clearTimers, | ||
| triggerLogout, | ||
| startCountdown, | ||
| timeoutMinutes, | ||
| warningMinutes, | ||
| ]) | ||
|
|
||
| // Handle "Stay signed in" button click | ||
| const handleStaySignedIn = useCallback(() => { | ||
| clearTimers() | ||
| setVisible(false) | ||
|
|
@@ -90,7 +119,6 @@ | |
| scheduleWarning() | ||
| }, [clearTimers, scheduleWarning]) | ||
|
|
||
| // Listen for activity from OTHER tabs via localStorage events | ||
| useEffect(() => { | ||
| const onStorage = (e: StorageEvent) => { | ||
| if (e.key !== 'lastActivity') return | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks to be in milliseconds not microseconds. Maybe rename it to
awayInMilliseconds?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha ha, yes, of course.