OCPBUGS-16633: Track API activity for inactivity timeout - #16851
OCPBUGS-16633: Track API activity for inactivity timeout#16851ericahinkleRH wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/severity-important
|
@ericahinkleRH: This pull request references Jira Issue OCPBUGS-16633, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ericahinkleRH The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds shared console activity tracking that stores the current timestamp in ChangesConsole activity tracking
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/packages/console-shared/src/utils/activity-tracker.ts`:
- Line 7: Update updateLastConsoleActivity to catch synchronous errors from
localStorage.setItem while preserving successful coFetch resolution; add a test
covering the storage-error case and verifying the activity update does not
reject.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 44bddb4a-51b6-46b9-947d-2efd1c4f8477
📒 Files selected for processing (2)
frontend/packages/console-shared/src/utils/activity-tracker.tsfrontend/packages/console-shared/src/utils/console-fetch.ts
| const LAST_CONSOLE_ACTIVITY_TIMESTAMP_LOCAL_STORAGE_KEY = 'last-console-activity-timestamp'; | ||
|
|
||
| export const updateLastConsoleActivity = (): void => { | ||
| localStorage.setItem(LAST_CONSOLE_ACTIVITY_TIMESTAMP_LOCAL_STORAGE_KEY, Date.now().toString()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File: activity-tracker.ts\n'
cat -n frontend/packages/console-shared/src/utils/activity-tracker.ts
printf '\n## Search references\n'
rg -n "LAST_CONSOLE_ACTIVITY_TIMESTAMP_LOCAL_STORAGE_KEY|activity-tracker|track.*Activity|setItem\\(LAST_CONSOLE_ACTIVITY_TIMESTAMP_LOCAL_STORAGE_KEY" frontend/packages/console-shared -S
printf '\n## Potential tests\n'
rg -n "localStorage|activity tracker|console activity|LAST_CONSOLE_ACTIVITY" frontend/packages/console-shared -S --glob '*.{test,spec}.{ts,tsx,js,jsx}'Repository: openshift/console
Length of output: 2042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' frontend/packages/console-shared/src/utils/activity-tracker.tsRepository: openshift/console
Length of output: 543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## console-fetch.ts\n'
cat -n frontend/packages/console-shared/src/utils/console-fetch.ts
printf '\n## Search for updateLastConsoleActivity call sites\n'
rg -n "updateLastConsoleActivity\\(" frontend/packages/console-shared -S
printf '\n## Search for existing localStorage failure handling patterns\n'
rg -n "try \\{|catch \\(|localStorage\\.setItem|localStorage\\.getItem" frontend/packages/console-shared/src -SRepository: openshift/console
Length of output: 11571
Catch storage errors in updateLastConsoleActivity. localStorage.setItem can throw synchronously when storage is blocked or full, and this call sits on the successful coFetch path. Catch that failure so successful responses still resolve, and add a test for the storage-error case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/packages/console-shared/src/utils/activity-tracker.ts` at line 7,
Update updateLastConsoleActivity to catch synchronous errors from
localStorage.setItem while preserving successful coFetch resolution; add a test
covering the storage-error case and verifying the activity update does not
reject.
The console inactivity timer was only tracking user clicks and keydown events, but not API requests. This caused premature logouts because: 1. Frontend timer measured time since last click/keydown 2. OAuth server measured time since last token usage (API requests) 3. Background API requests (resource watches, polling) extended the OAuth server's token timeout 4. Frontend timer fired first and explicitly deleted the token before the OAuth server would have expired it naturally This fix tracks API activity by updating the last activity timestamp on every successful API request. The existing storage event listener detects these updates and resets the inactivity timer, keeping the frontend timer in sync with actual API activity. Changes: - Created activity-tracker.ts to centralize activity tracking - Updated coFetch() to call updateLastConsoleActivity() after each successful API request - Existing onStorageChange listener in masthead-toolbar.tsx handles the timestamp updates and resets the timeout This aligns the frontend inactivity detection with the OAuth server's behavior, preventing tokens from being deleted while they're still actively being used by background API requests.
f4a5c56 to
a070c6b
Compare
|
@ericahinkleRH: This pull request references Jira Issue OCPBUGS-16633, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@ericahinkleRH: This pull request references Jira Issue OCPBUGS-16633, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (yapei@redhat.com), skipping review request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/ok-to-test |
openshift-ci
Bot
added
the
ok-to-test
|
@ericahinkleRH: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Problem
Console client sessions are being deleted before the OAuth server's
inactivityTimeoutSecondsexpires. This happens because two independent timers race each other:The frontend timer fires first and explicitly deletes the token before the OAuth server would naturally expire it, causing premature logouts.
Root Cause
Identified by @rluby in the JIRA:
inactivityTimeoutSecondsSolution
Track API activity by updating the last activity timestamp on every successful API request. This keeps the frontend inactivity timer in sync with actual API usage.
Changes:
activity-tracker.tsto centralize activity tracking via localStoragecoFetch()to callupdateLastConsoleActivity()after each successful requestonStorageChangelistener inmasthead-toolbar.tsxdetects updates and resets the timerThis aligns frontend inactivity detection with the OAuth server's behavior.
Testing
Manual testing needed with
accessTokenInactivityTimeoutSeconds: 300set on console OAuthClient:inactivityTimeoutSecondscontinues to be extended by API activityFixes: https://issues.redhat.com/browse/OCPBUGS-16633
Summary by CodeRabbit