UI: don’t continue with session with an expired userid - #13969
Conversation
|
@prashanthr2 please have a look. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13969 +/- ##
============================================
+ Coverage 16.32% 16.37% +0.05%
- Complexity 13556 13602 +46
============================================
Files 5669 5669
Lines 501399 501446 +47
Branches 60902 60913 +11
============================================
+ Hits 81847 82113 +266
+ Misses 410390 410129 -261
- Partials 9162 9204 +42
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8c7d95d to
90d11d8
Compare
|
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
This PR aims to prevent the UI from continuing with a cached auth/session state when the userid cookie has expired, which previously could misrepresent the active identity in the UI.
Changes:
- Refactors duplicated login response handling into shared helper functions.
- Adds a guard when reusing cached auth state to reject if
useridcookie is missing.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function setLoginCookies (result) { | ||
| Cookies.set('account', result.account) | ||
| Cookies.set('domainid', result.domainid) | ||
| Cookies.set('role', result.type) | ||
| Cookies.set('timezone', result.timezone) | ||
| Cookies.set('timezoneoffset', result.timezoneoffset) | ||
| Cookies.set('userfullname', result.firstname + ' ' + result.lastname) | ||
| Cookies.set('userid', result.userid) | ||
| Cookies.set('username', result.username) |
| if (hasAuth) { | ||
| if (!Cookies.get('userid')) { | ||
| reject(new Error('Identity session expired')) | ||
| return | ||
| } |
| commit('SET_LATEST_VERSION', latestVersion) | ||
| if (hasAuth) { | ||
| if (!Cookies.get('userid')) { | ||
| reject(new Error('Identity session expired')) |
sudo87
left a comment
sudo87
left a comment
There was a problem hiding this comment.
lgtm, tested basic login-logout flow
shwstppr
left a comment
shwstppr
left a comment
There was a problem hiding this comment.
LGTM. Tested
I used a cookie expiry of 75s with 4.20 branch and changes from the PR
Before fix, logged in user gets switched to the first user of the admin account on cookie expiry
before-ui-userswitch-cookieexpiry.mp4
After fix, ui is moved to login screen on cookie expiry
after-ui-userswitch-cookieexpiry.mp4
Description
This PR fixes a representational issue where a member of the root admin account would think they had now become user “admin” after their cookies would expire. Any action they would take would still be accounted to their own userid as can be shown by events.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?